Fix handling of R class in optimized shrinking
With this change we will optimize R class fields, even when optimization is off.
This basically means that we will remove fields that is only written
in clinit, and have no other reads or writes.
To enable this, we now do deferred tracing in the first round of tree shaking.
Bug: b/336983087
Bug: b/325905703
Bug: b/287398085
Change-Id: I7dd72f1e9619340618d406876601cd1f2ea00f55
diff --git a/src/main/java/com/android/tools/r8/cf/code/CfFrameVerifier.java b/src/main/java/com/android/tools/r8/cf/code/CfFrameVerifier.java
index d1b3d3e..86ea3c0 100644
--- a/src/main/java/com/android/tools/r8/cf/code/CfFrameVerifier.java
+++ b/src/main/java/com/android/tools/r8/cf/code/CfFrameVerifier.java
@@ -248,29 +248,32 @@
// safe assuming an incoming type state T. The type state T is derived from ExcStackFrame
// by replacing the operand stack with a stack whose sole element is the handler's
// exception class.
- for (CfLabel target : tryCatchRange.getTargets()) {
+ List<CfLabel> targets = tryCatchRange.getTargets();
+ for (int i = 0; i < targets.size(); i++) {
+ CfLabel target = targets.get(i);
+ DexType guard = tryCatchRange.guards.get(i);
CfFrame destinationFrame = labelToFrameMap.get(target);
if (destinationFrame == null) {
return CfCodeStackMapValidatingException.invalidTryCatchRange(
method, tryCatchRange, "No frame for target catch range target", appView);
}
- // From the spec: the handler's exception class is assignable to the class Throwable.
- for (DexType guard : tryCatchRange.guards) {
- if (!config.getAssignability().isAssignable(guard, factory.throwableType)) {
- return CfCodeStackMapValidatingException.invalidTryCatchRange(
- method,
- tryCatchRange,
- "Could not assign " + guard.getTypeName() + " to java.lang.Throwable",
- appView);
- }
- Deque<PreciseFrameType> sourceStack =
- ImmutableDeque.of(FrameType.initializedNonNullReference(guard));
- AssignabilityResult assignabilityResult =
- config.getAssignability().isStackAssignable(sourceStack, destinationFrame.getStack());
- if (assignabilityResult.isFailed()) {
- return CfCodeStackMapValidatingException.invalidTryCatchRange(
- method, tryCatchRange, assignabilityResult.asFailed().getMessage(), appView);
- }
+ Deque<PreciseFrameType> sourceStack =
+ ImmutableDeque.of(FrameType.initializedNonNullReference(guard));
+ AssignabilityResult assignabilityResult =
+ config.getAssignability().isStackAssignable(sourceStack, destinationFrame.getStack());
+ if (assignabilityResult.isFailed()) {
+ return CfCodeStackMapValidatingException.invalidTryCatchRange(
+ method, tryCatchRange, assignabilityResult.asFailed().getMessage(), appView);
+ }
+ }
+ // From the spec: the handler's exception class is assignable to the class Throwable.
+ for (DexType guard : tryCatchRange.guards) {
+ if (!config.getAssignability().isAssignable(guard, factory.throwableType)) {
+ return CfCodeStackMapValidatingException.invalidTryCatchRange(
+ method,
+ tryCatchRange,
+ "Could not assign " + guard.getTypeName() + " to java.lang.Throwable",
+ appView);
}
}
return null;
diff --git a/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracing.java b/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracing.java
index 78abb50..51e854f 100644
--- a/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracing.java
+++ b/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracing.java
@@ -21,13 +21,11 @@
public static EnqueuerDeferredTracing create(
AppView<? extends AppInfoWithClassHierarchy> appView, Enqueuer enqueuer, Mode mode) {
- if (mode.isInitialTreeShaking()) {
+ InternalOptions options = appView.options();
+ if (!options.isShrinking() || !options.enableEnqueuerDeferredTracing) {
return empty();
}
- InternalOptions options = appView.options();
- if (!options.isOptimizing()
- || !options.isShrinking()
- || !options.enableEnqueuerDeferredTracing) {
+ if (!options.isOptimizing() && !options.isOptimizedResourceShrinking()) {
return empty();
}
return new EnqueuerDeferredTracingImpl(appView, enqueuer, mode);
diff --git a/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracingImpl.java b/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracingImpl.java
index 1eacba7..5291978 100644
--- a/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracingImpl.java
+++ b/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracingImpl.java
@@ -128,6 +128,11 @@
// field's holder. Therefore, we unconditionally trace the class initializer in this case.
// The corresponding IR rewriter will rewrite the field access into an init-class instruction.
if (accessKind.isStatic()) {
+ if (enqueuer.getMode().isInitialTreeShaking() && field.getHolder() != context.getHolder()) {
+ // TODO(b/338000616): No support for InitClass until we have LIR in the initial round of
+ // tree shaking.
+ return false;
+ }
KeepReason reason =
enqueuer.getGraphReporter().reportClassReferencedFrom(field.getHolder(), context);
enqueuer.getWorklist().enqueueTraceTypeReferenceAction(field.getHolder(), reason);
@@ -148,7 +153,7 @@
}
assert enqueuer.getKeepInfo(field).isBottom();
- assert !enqueuer.getKeepInfo(field).isPinned(options);
+ assert !enqueuer.getKeepInfo(field).isPinned(options) || options.isOptimizedResourceShrinking();
FieldAccessInfo info = enqueuer.getFieldAccessInfoCollection().get(field.getReference());
if (info.hasReflectiveAccess()
@@ -162,6 +167,12 @@
|| !minimumKeepInfo.isShrinkingAllowed())) {
return false;
}
+ if (!options.isOptimizing()) {
+ assert options.isOptimizedResourceShrinking();
+ if (!enqueuer.isRClass(field.getHolder())) {
+ return false;
+ }
+ }
if (info.isWritten()) {
// If the assigned value may have an override of Object#finalize() then give up.
diff --git a/src/test/java/com/android/tools/r8/androidresources/NoOptResourceShrinkingTest.java b/src/test/java/com/android/tools/r8/androidresources/NoOptResourceShrinkingTest.java
index e428d08..2248d93 100644
--- a/src/test/java/com/android/tools/r8/androidresources/NoOptResourceShrinkingTest.java
+++ b/src/test/java/com/android/tools/r8/androidresources/NoOptResourceShrinkingTest.java
@@ -55,13 +55,8 @@
resourceTableInspector -> {
resourceTableInspector.assertContainsResourceWithName("string", "bar");
resourceTableInspector.assertContainsResourceWithName("string", "foo");
- if (optimized) {
- // TODO(b/336983087): This should be removed.
- resourceTableInspector.assertContainsResourceWithName("string", "unused_string");
- } else {
- resourceTableInspector.assertDoesNotContainResourceWithName(
- "string", "unused_string");
- }
+ resourceTableInspector.assertDoesNotContainResourceWithName(
+ "string", "unused_string");
})
.run(parameters.getRuntime(), FooBar.class)
.assertSuccess();
diff --git a/src/test/java/com/android/tools/r8/androidresources/optimizedshrinking/TestOptimizedShrinking.java b/src/test/java/com/android/tools/r8/androidresources/optimizedshrinking/TestOptimizedShrinking.java
index 74c7102..584426b 100644
--- a/src/test/java/com/android/tools/r8/androidresources/optimizedshrinking/TestOptimizedShrinking.java
+++ b/src/test/java/com/android/tools/r8/androidresources/optimizedshrinking/TestOptimizedShrinking.java
@@ -107,7 +107,7 @@
// In optimized mode we track these correctly, so we should not unconditionally keep
// all attributes.
- if (optimized && !debug) {
+ if (optimized) {
resourceTableInspector.assertDoesNotContainResourceWithName(
"attr", "attr_unused_styleable" + i);
} else {
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/EmptyEnumUnboxingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/EmptyEnumUnboxingTest.java
index c2ab4e2..2cbfd3f 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/EmptyEnumUnboxingTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/EmptyEnumUnboxingTest.java
@@ -38,8 +38,16 @@
.addInnerClasses(EmptyEnumUnboxingTest.class)
.addKeepMainRule(Main.class)
.addKeepRules(enumKeepRules.getKeepRules())
- // TODO(b/166532373): Unbox enum with no cases.
- .addEnumUnboxingInspector(inspector -> inspector.assertNotUnboxed(MyEnum.class))
+ .addEnumUnboxingInspector(
+ inspector -> {
+ if (enumKeepRules.isStudio()) {
+ // TODO(b/166532373): Unbox enum with no cases.
+ inspector.assertNotUnboxed(MyEnum.class);
+ } else {
+ assert enumKeepRules.isNone();
+ inspector.assertUnboxed(MyEnum.class);
+ }
+ })
.enableNeverClassInliningAnnotations()
.enableInliningAnnotations()
.addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
