Synthesize InitClass instruction in inliner
Change-Id: I8781141485e1b3d942610377c4649ea88618f770
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index f1c5dc9..efdf473 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -128,62 +128,6 @@
return false;
}
- private boolean canInlineStaticInvoke(
- InvokeStatic invoke,
- DexEncodedMethod method,
- DexEncodedMethod target,
- ClassInitializationAnalysis classInitializationAnalysis,
- WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
- // Only proceed with inlining a static invoke if:
- // - the holder for the target is a subtype of the holder for the method,
- // - the target method always triggers class initialization of its holder before any other side
- // effect (hence preserving class initialization semantics),
- // - the current method has already triggered the holder for the target method to be
- // initialized, or
- // - there is no non-trivial class initializer.
- DexType targetHolder = target.method.holder;
- if (appView.appInfo().isSubtype(method.method.holder, targetHolder)) {
- return true;
- }
- DexClass clazz = appView.definitionFor(targetHolder);
- assert clazz != null;
- if (target.getOptimizationInfo().triggersClassInitBeforeAnySideEffect()) {
- return true;
- }
- if (!method.isStatic()) {
- boolean targetIsGuaranteedToBeInitialized =
- appView.withInitializedClassesInInstanceMethods(
- analysis ->
- analysis.isClassDefinitelyLoadedInInstanceMethodsOn(
- target.method.holder, method.method.holder),
- false);
- if (targetIsGuaranteedToBeInitialized) {
- return true;
- }
- }
- if (classInitializationAnalysis.isClassDefinitelyLoadedBeforeInstruction(
- target.method.holder, invoke)) {
- return true;
- }
- // Check for class initializer side effects when loading this class, as inlining might remove
- // the load operation.
- //
- // See https://docs.oracle.com/javase/specs/jvms/se9/html/jvms-5.html#jvms-5.5.
- //
- // For simplicity, we are conservative and consider all interfaces, not only the ones with
- // default methods.
- if (!clazz.classInitializationMayHaveSideEffects(appView)) {
- return true;
- }
-
- if (appView.rootSet().bypassClinitForInlining.contains(target.method)) {
- return true;
- }
-
- whyAreYouNotInliningReporter.reportMustTriggerClassInitialization();
- return false;
- }
-
@Override
public boolean passesInliningConstraints(
InvokeMethod invoke,
@@ -391,12 +335,70 @@
Reason reason,
ClassInitializationAnalysis classInitializationAnalysis,
WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
- // Abort inlining attempt if we can not guarantee class for static target has been initialized.
- if (!canInlineStaticInvoke(
- invoke, method, singleTarget, classInitializationAnalysis, whyAreYouNotInliningReporter)) {
- return null;
+ InlineAction action = new InlineAction(singleTarget, invoke, reason);
+ if (isTargetClassInitialized(invoke, method, singleTarget, classInitializationAnalysis)) {
+ return action;
}
- return new InlineAction(singleTarget, invoke, reason);
+ if (appView.canUseInitClass()) {
+ action.setShouldSynthesizeInitClass();
+ return action;
+ }
+ whyAreYouNotInliningReporter.reportMustTriggerClassInitialization();
+ return null;
+ }
+
+ private boolean isTargetClassInitialized(
+ InvokeStatic invoke,
+ DexEncodedMethod method,
+ DexEncodedMethod target,
+ ClassInitializationAnalysis classInitializationAnalysis) {
+ // Only proceed with inlining a static invoke if:
+ // - the holder for the target is a subtype of the holder for the method,
+ // - the target method always triggers class initialization of its holder before any other side
+ // effect (hence preserving class initialization semantics),
+ // - the current method has already triggered the holder for the target method to be
+ // initialized, or
+ // - there is no non-trivial class initializer.
+ DexType targetHolder = target.method.holder;
+ if (appView.appInfo().isSubtype(method.method.holder, targetHolder)) {
+ return true;
+ }
+ DexClass clazz = appView.definitionFor(targetHolder);
+ assert clazz != null;
+ if (target.getOptimizationInfo().triggersClassInitBeforeAnySideEffect()) {
+ return true;
+ }
+ if (!method.isStatic()) {
+ boolean targetIsGuaranteedToBeInitialized =
+ appView.withInitializedClassesInInstanceMethods(
+ analysis ->
+ analysis.isClassDefinitelyLoadedInInstanceMethodsOn(
+ target.method.holder, method.method.holder),
+ false);
+ if (targetIsGuaranteedToBeInitialized) {
+ return true;
+ }
+ }
+ if (classInitializationAnalysis.isClassDefinitelyLoadedBeforeInstruction(
+ target.method.holder, invoke)) {
+ return true;
+ }
+ // Check for class initializer side effects when loading this class, as inlining might remove
+ // the load operation.
+ //
+ // See https://docs.oracle.com/javase/specs/jvms/se9/html/jvms-5.html#jvms-5.5.
+ //
+ // For simplicity, we are conservative and consider all interfaces, not only the ones with
+ // default methods.
+ if (!clazz.classInitializationMayHaveSideEffects(appView)) {
+ return true;
+ }
+
+ if (appView.rootSet().bypassClinitForInlining.contains(target.method)) {
+ return true;
+ }
+
+ return false;
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index c9bf058..041d5a8 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -26,6 +26,7 @@
import com.android.tools.r8.ir.code.CatchHandlers.CatchHandler;
import com.android.tools.r8.ir.code.ConstClass;
import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.InitClass;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionIterator;
import com.android.tools.r8.ir.code.InstructionListIterator;
@@ -573,6 +574,7 @@
public final Invoke invoke;
final Reason reason;
+ private boolean shouldSynthesizeInitClass;
private boolean shouldSynthesizeNullCheckForReceiver;
InlineAction(DexEncodedMethod target, Invoke invoke, Reason reason) {
@@ -581,7 +583,13 @@
this.reason = reason;
}
+ void setShouldSynthesizeInitClass() {
+ assert !shouldSynthesizeNullCheckForReceiver;
+ shouldSynthesizeInitClass = true;
+ }
+
void setShouldSynthesizeNullCheckForReceiver() {
+ assert !shouldSynthesizeInitClass;
shouldSynthesizeNullCheckForReceiver = true;
}
@@ -599,6 +607,12 @@
// Build the IR for a yet not processed method, and perform minimal IR processing.
IRCode code = inliningIRProvider.getInliningIR(invoke, target);
+ // Insert a init class instruction if this is needed to preserve class initialization
+ // semantics.
+ if (shouldSynthesizeInitClass) {
+ synthesizeInitClass(code);
+ }
+
// Insert a null check if this is needed to preserve the implicit null check for the receiver.
// This is only needed if we do not also insert a monitor-enter instruction, since that will
// throw a NPE if the receiver is null.
@@ -737,6 +751,21 @@
return new InlineeWithReason(code, reason);
}
+ private void synthesizeInitClass(IRCode code) {
+ List<Value> arguments = code.collectArguments();
+ BasicBlock entryBlock = code.entryBlock();
+
+ // Insert a new block between the last argument instruction and the first actual instruction
+ // of the method.
+ BasicBlock initClassBlock =
+ entryBlock.listIterator(code, arguments.size()).split(code, 0, null);
+ assert !initClassBlock.hasCatchHandlers();
+
+ InstructionListIterator iterator = initClassBlock.listIterator(code);
+ iterator.setInsertionPosition(entryBlock.exit().getPosition());
+ iterator.add(new InitClass(code.createValue(TypeLatticeElement.getInt()), target.holder()));
+ }
+
private void synthesizeNullCheckForReceiver(AppView<?> appView, IRCode code) {
List<Value> arguments = code.collectArguments();
if (!arguments.isEmpty()) {
diff --git a/src/test/java/com/android/tools/r8/classmerging/InliningAfterStaticClassMergerTest.java b/src/test/java/com/android/tools/r8/classmerging/InliningAfterStaticClassMergerTest.java
index 23c2d55..5fdd52b 100644
--- a/src/test/java/com/android/tools/r8/classmerging/InliningAfterStaticClassMergerTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/InliningAfterStaticClassMergerTest.java
@@ -102,9 +102,18 @@
FoundClassSubject clazz = classes.get(0);
assertEquals(StaticMergeCandidateA.class.getTypeName(), clazz.getOriginalName());
- // Check that StaticMergeCandidateB.m() has not been moved into StaticMergeCandidateA, because
- // that would disable inlining of it.
- assertEquals(1, clazz.allMethods().size());
+ if (parameters.isCfRuntime()) {
+ // Check that StaticMergeCandidateB.m() has not been moved into StaticMergeCandidateA, because
+ // that would disable inlining of it.
+ assertEquals(1, clazz.allMethods().size());
+ } else {
+ // Check that StaticMergeCandidateB.m() has been inlined.
+ assertEquals(0, clazz.allMethods().size());
+
+ // Check that a static field has been synthesized in order to trigger class initialization.
+ assertEquals(1, clazz.allStaticFields().size());
+ assertEquals("$r8$clinit", clazz.allStaticFields().get(0).getOriginalName());
+ }
// Check that the test class only has a main method.
ClassSubject classSubject = inspector.clazz(TestClass.class);
diff --git a/src/test/java/com/android/tools/r8/graph/initializedclasses/InitializedClassesInInstanceMethodsTest.java b/src/test/java/com/android/tools/r8/graph/initializedclasses/InitializedClassesInInstanceMethodsTest.java
index 43015bf..ee86dc8 100644
--- a/src/test/java/com/android/tools/r8/graph/initializedclasses/InitializedClassesInInstanceMethodsTest.java
+++ b/src/test/java/com/android/tools/r8/graph/initializedclasses/InitializedClassesInInstanceMethodsTest.java
@@ -30,7 +30,8 @@
@Parameters(name = "{1}, enabled: {0}")
public static List<Object[]> data() {
- return buildParameters(BooleanUtils.values(), getTestParameters().withAllRuntimes().build());
+ return buildParameters(
+ BooleanUtils.values(), getTestParameters().withAllRuntimesAndApiLevels().build());
}
public InitializedClassesInInstanceMethodsTest(
@@ -53,7 +54,7 @@
.allowAccessModification()
.enableNeverClassInliningAnnotations()
.enableInliningAnnotations()
- .setMinApi(parameters.getRuntime())
+ .setMinApi(parameters.getApiLevel())
.compile()
.inspect(this::verifyOutput)
.run(parameters.getRuntime(), TestClass.class)
@@ -71,12 +72,15 @@
assertThat(outerClassSubject.uniqueMethodWithName("exclamationMark"), not(isPresent()));
int numberOfExpectedAccessibilityBridges =
- enableInitializedClassesInInstanceMethodsAnalysis ? 0 : 3;
+ enableInitializedClassesInInstanceMethodsAnalysis || parameters.isDexRuntime() ? 0 : 3;
assertEquals(
numberOfExpectedAccessibilityBridges,
outerClassSubject
.allMethods(method -> method.getOriginalName().contains("access$"))
.size());
+ assertEquals(
+ parameters.isDexRuntime() && !enableInitializedClassesInInstanceMethodsAnalysis,
+ outerClassSubject.uniqueFieldWithName("$r8$clinit").isPresent());
ClassSubject aClassSubject = inspector.clazz(Outer.A.class);
assertThat(aClassSubject, isPresent());
diff --git a/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/SingletonClassInitializerPatternCannotBePostponedTest.java b/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/SingletonClassInitializerPatternCannotBePostponedTest.java
index d29753b..4213323 100644
--- a/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/SingletonClassInitializerPatternCannotBePostponedTest.java
+++ b/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/SingletonClassInitializerPatternCannotBePostponedTest.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.ir.analysis.sideeffect;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.TestBase;
@@ -47,9 +48,16 @@
ClassSubject classSubject = inspector.clazz(A.class);
assertThat(classSubject, isPresent());
- // A.inlineable() cannot be inlined because it should trigger the class initialization of A,
- // which should trigger the class initialization of B, which will print "Hello".
- assertThat(classSubject.uniqueMethodWithName("inlineable"), isPresent());
+ if (parameters.isCfRuntime()) {
+ // A.inlineable() cannot be inlined because it should trigger the class initialization of A,
+ // which should trigger the class initialization of B, which will print "Hello".
+ assertThat(classSubject.uniqueFieldWithName("$r8$clinit"), not(isPresent()));
+ assertThat(classSubject.uniqueMethodWithName("inlineable"), isPresent());
+ } else {
+ // A static field A.$r8$clinit has been synthesized to allow inlining of A.inlineable().
+ assertThat(classSubject.uniqueFieldWithName("$r8$clinit"), isPresent());
+ assertThat(classSubject.uniqueMethodWithName("inlineable"), not(isPresent()));
+ }
}
static class TestClass {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InliningFromCurrentClassTest.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InliningFromCurrentClassTest.java
index 239b944..27f3735 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InliningFromCurrentClassTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InliningFromCurrentClassTest.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.ir.optimize.inliner;
+import static com.android.tools.r8.utils.codeinspector.CodeMatchers.accessesField;
import static com.android.tools.r8.utils.codeinspector.CodeMatchers.invokesMethod;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
@@ -12,14 +13,30 @@
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NeverMerge;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+@RunWith(Parameterized.class)
public class InliningFromCurrentClassTest extends TestBase {
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public InliningFromCurrentClassTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
@Test
public void test() throws Exception {
String expectedOutput =
@@ -29,17 +46,18 @@
"In A.inlineable1()",
"In B.inlineable2()",
"In C.<clinit>()",
- "In C.notInlineable()");
+ "In C.inlineableWithInitClass()");
testForJvm().addTestClasspath().run(TestClass.class).assertSuccessWithOutput(expectedOutput);
CodeInspector inspector =
- testForR8(Backend.DEX)
+ testForR8(parameters.getBackend())
.addInnerClasses(InliningFromCurrentClassTest.class)
.addKeepMainRule(TestClass.class)
.enableInliningAnnotations()
.enableMergeAnnotations()
- .run(TestClass.class)
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), TestClass.class)
.assertSuccessWithOutput(expectedOutput)
.inspector();
@@ -52,18 +70,24 @@
ClassSubject classC = inspector.clazz(C.class);
assertThat(classC, isPresent());
+ MethodSubject testMethod = classB.uniqueMethodWithName("test");
+ assertThat(testMethod, isPresent());
+
MethodSubject inlineable1Method = classA.uniqueMethodWithName("inlineable1");
assertThat(inlineable1Method, not(isPresent()));
MethodSubject inlineable2Method = classB.uniqueMethodWithName("inlineable2");
assertThat(inlineable2Method, not(isPresent()));
- MethodSubject notInlineableMethod = classC.uniqueMethodWithName("notInlineable");
- assertThat(notInlineableMethod, isPresent());
-
- MethodSubject testMethod = classB.uniqueMethodWithName("test");
- assertThat(testMethod, isPresent());
- assertThat(testMethod, invokesMethod(notInlineableMethod));
+ MethodSubject inlineableWithInitClassMethod =
+ classC.uniqueMethodWithName("inlineableWithInitClass");
+ if (parameters.isCfRuntime()) {
+ assertThat(inlineableWithInitClassMethod, isPresent());
+ assertThat(testMethod, invokesMethod(inlineableWithInitClassMethod));
+ } else {
+ assertThat(inlineableWithInitClassMethod, not(isPresent()));
+ assertThat(testMethod, accessesField(classC.uniqueFieldWithName("$r8$clinit")));
+ }
}
static class TestClass {
@@ -96,7 +120,7 @@
static void test() {
A.inlineable1();
B.inlineable2();
- C.notInlineable();
+ C.inlineableWithInitClass();
}
static void inlineable2() {
@@ -110,8 +134,8 @@
System.out.println("In C.<clinit>()");
}
- static void notInlineable() {
- System.out.println("In C.notInlineable()");
+ static void inlineableWithInitClass() {
+ System.out.println("In C.inlineableWithInitClass()");
}
}
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeMatchers.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeMatchers.java
index 5dacc5a..86a21c1 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeMatchers.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeMatchers.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.utils.codeinspector;
+import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexMethod;
import java.util.function.Predicate;
import org.hamcrest.Description;
@@ -12,6 +13,35 @@
public class CodeMatchers {
+ public static Matcher<MethodSubject> accessesField(FieldSubject targetSubject) {
+ if (!targetSubject.isPresent()) {
+ throw new IllegalArgumentException();
+ }
+ DexField target = targetSubject.getField().field;
+ return new TypeSafeMatcher<MethodSubject>() {
+ @Override
+ protected boolean matchesSafely(MethodSubject subject) {
+ if (!subject.isPresent()) {
+ return false;
+ }
+ if (!subject.getMethod().hasCode()) {
+ return false;
+ }
+ return subject.streamInstructions().anyMatch(isFieldAccessWithTarget(target));
+ }
+
+ @Override
+ public void describeTo(Description description) {
+ description.appendText("accesses field `" + target.toSourceString() + "`");
+ }
+
+ @Override
+ public void describeMismatchSafely(final MethodSubject subject, Description description) {
+ description.appendText("method did not");
+ }
+ };
+ }
+
public static Matcher<MethodSubject> invokesMethod(MethodSubject targetSubject) {
if (!targetSubject.isPresent()) {
throw new IllegalArgumentException();
@@ -44,4 +74,8 @@
public static Predicate<InstructionSubject> isInvokeWithTarget(DexMethod target) {
return instruction -> instruction.isInvoke() && instruction.getMethod() == target;
}
+
+ public static Predicate<InstructionSubject> isFieldAccessWithTarget(DexField target) {
+ return instruction -> instruction.isFieldAccess() && instruction.getField() == target;
+ }
}
