Final field constructor inlining without storeStoreFence
This adds a new system property com.android.tools.r8.skipStoreStoreFenceInConstructorInlining, which defaults to false.
When the system property is set, constructors that assign final fields become subject to inlining all the way back to min API 21, instead of min API 33, since the inliner will not emit a call to VarHandle.storeStoreFence() at the constructor exit upon inlining.
Note: Inlining of constructors that assign final fields is still disabled by default. This can be enabled by setting the system property com.android.tools.r8.enableConstructorInliningWithFinalFields.
Bug: b/416714948
Change-Id: I8db6495d174ad42b8d148aedbc7498576a424a97
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 a593c4a..1711903 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
@@ -752,7 +752,8 @@
}
if (target.getAccessFlags().isFinal()) {
if (inlinerOptions.enableConstructorInliningWithFinalFields
- && options.canUseJavaLangVarHandleStoreStoreFence(appView)
+ && (options.canUseJavaLangVarHandleStoreStoreFence(appView)
+ || inlinerOptions.skipStoreStoreFenceInConstructorInlining)
&& methodProcessor.hasWaves()
&& target.isProgramField()) {
actionBuilder.setShouldEnsureStoreStoreFence(target.asProgramField());
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 995b1bf..e291186 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
@@ -644,7 +644,8 @@
InvokeDirect invokeDirect = invoke.asInvokeDirect();
Value receiver = invokeDirect.getReceiver();
if (shouldEnsureStoreStoreFenceCauses != null
- && receiver.isDefinedByInstructionSatisfying(Instruction::isNewInstance)) {
+ && receiver.isDefinedByInstructionSatisfying(Instruction::isNewInstance)
+ && !options.inlinerOptions().skipStoreStoreFenceInConstructorInlining) {
synthesizeStoreStoreFence(appView, code, invokeDirect);
}
}
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 60ee0bd..6125bba 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -1833,6 +1833,9 @@
public boolean enableConstructorInliningWithFinalFields =
parseSystemPropertyOrDefault(
"com.android.tools.r8.enableConstructorInliningWithFinalFields", false);
+ public boolean skipStoreStoreFenceInConstructorInlining =
+ parseSystemPropertyOrDefault(
+ "com.android.tools.r8.skipStoreStoreFenceInConstructorInlining", false);
public boolean enableInlining =
!parseSystemPropertyForDevelopmentOrDefault("com.android.tools.r8.disableinlining", false);
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineConstructorWithFinalFieldsTest.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineConstructorWithFinalFieldsTest.java
index 6e8e2f9..c899dcb 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineConstructorWithFinalFieldsTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineConstructorWithFinalFieldsTest.java
@@ -7,19 +7,21 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsentIf;
import static com.android.tools.r8.utils.codeinspector.Matchers.isFinal;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.notIf;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.AlwaysInline;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.references.Reference;
+import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.MethodReferenceUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.FieldSubject;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -32,9 +34,13 @@
@Parameter(0)
public TestParameters parameters;
- @Parameters(name = "{0}")
- public static TestParametersCollection data() {
- return getTestParameters().withAllRuntimesAndApiLevels().build();
+ @Parameter(1)
+ public boolean skipStoreStoreFenceInConstructorInlining;
+
+ @Parameters(name = "{0}, skip: {1}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withAllRuntimesAndApiLevels().build(), BooleanUtils.values());
}
@Test
@@ -46,6 +52,10 @@
parameters.isDexRuntime(),
testBuilder -> testBuilder.addLibraryFiles(ToolHelper.getMostRecentAndroidJar()))
.addKeepMainRule(Main.class)
+ .addOptionsModification(
+ options ->
+ options.inlinerOptions().skipStoreStoreFenceInConstructorInlining =
+ skipStoreStoreFenceInConstructorInlining)
.enableAlwaysInliningAnnotations()
.setMinApi(parameters)
.compile()
@@ -63,7 +73,10 @@
MethodSubject initMethodSubject = mainClassSubject.init("int", "int");
assertThat(
initMethodSubject,
- isAbsentIf(parameters.canUseJavaLangInvokeVarHandleStoreStoreFence()));
+ isAbsentIf(
+ parameters.canUseJavaLangInvokeVarHandleStoreStoreFence()
+ || (parameters.canInitNewInstanceUsingSuperclassConstructor()
+ && skipStoreStoreFenceInConstructorInlining)));
MethodSubject mainMethodSubject = mainClassSubject.mainMethod();
assertThat(mainMethodSubject, isPresent());
@@ -78,9 +91,11 @@
invokesMethod(MethodReferenceUtils.instanceConstructor(Object.class)));
assertThat(
mainMethodSubject,
- invokesMethod(
- Reference.methodFromDescriptor(
- "Ljava/lang/invoke/VarHandle;", "storeStoreFence", "()V")));
+ notIf(
+ invokesMethod(
+ Reference.methodFromDescriptor(
+ "Ljava/lang/invoke/VarHandle;", "storeStoreFence", "()V")),
+ skipStoreStoreFenceInConstructorInlining));
assertThat(xFieldSubject, not(isFinal()));
assertThat(yFieldSubject, not(isFinal()));
}
