Optimize AtomicIntegerFieldUpdater.get.
Bug: b/453628974
Change-Id: I99618904e5107056d8a5856dd86443cc606792f4
diff --git a/src/main/java/com/android/tools/r8/androidapi/AndroidApiLevelDatabaseHelper.java b/src/main/java/com/android/tools/r8/androidapi/AndroidApiLevelDatabaseHelper.java
index 59c3096..6b4cdbe 100644
--- a/src/main/java/com/android/tools/r8/androidapi/AndroidApiLevelDatabaseHelper.java
+++ b/src/main/java/com/android/tools/r8/androidapi/AndroidApiLevelDatabaseHelper.java
@@ -577,12 +577,7 @@
factory.createProto(factory.intType, factory.objectType, factory.longType),
"getInt"),
always);
- apiLevelConsumer.accept(
- factory.createMethod(
- factory.sunMiscUnsafeType,
- factory.createProto(factory.intType, factory.objectType, factory.longType),
- "getIntVolatile"),
- always);
+ apiLevelConsumer.accept(factory.sunMiscUnsafeMethods.getIntVolatile, always);
apiLevelConsumer.accept(
factory.createMethod(
factory.sunMiscUnsafeType,
@@ -607,12 +602,7 @@
factory.createProto(factory.objectType, factory.objectType, factory.longType),
"getObject"),
always);
- apiLevelConsumer.accept(
- factory.createMethod(
- factory.sunMiscUnsafeType,
- factory.createProto(factory.objectType, factory.objectType, factory.longType),
- "getObjectVolatile"),
- always);
+ apiLevelConsumer.accept(factory.sunMiscUnsafeMethods.getObjectVolatile, always);
apiLevelConsumer.accept(
factory.createMethod(
factory.sunMiscUnsafeType,
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index cb950bd..e9ca9ab 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -2593,6 +2593,7 @@
public class AtomicIntUpdaterMethods {
public final DexMethod newUpdater;
public final DexMethod compareAndSet;
+ public final DexMethod get;
private AtomicIntUpdaterMethods() {
newUpdater =
@@ -2607,6 +2608,12 @@
compareAndSetName,
booleanDescriptor,
new DexString[] {objectDescriptor, intDescriptor, intDescriptor});
+ get =
+ createMethod(
+ intFieldUpdaterDescriptor,
+ getName,
+ intDescriptor,
+ new DexString[] {objectDescriptor});
}
}
@@ -2675,6 +2682,8 @@
public final DexMethod compareAndSwapInt;
public final DexMethod compareAndSwapObject;
public final DexMethod objectFieldOffset;
+ public final DexMethod getObjectVolatile;
+ public final DexMethod getIntVolatile;
private SunMiscUnsafeMethods() {
this.compareAndSwapInt =
@@ -2689,6 +2698,14 @@
"compareAndSwapObject");
this.objectFieldOffset =
createMethod(sunMiscUnsafeType, createProto(longType, fieldType), "objectFieldOffset");
+ this.getObjectVolatile =
+ createMethod(
+ sunMiscUnsafeType,
+ createProto(objectType, objectType, longType),
+ "getObjectVolatile");
+ this.getIntVolatile =
+ createMethod(
+ sunMiscUnsafeType, createProto(intType, objectType, longType), "getIntVolatile");
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/AtomicFieldUpdaterOptimizer.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/AtomicFieldUpdaterOptimizer.java
index 73863ae..26df724 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/passes/AtomicFieldUpdaterOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/AtomicFieldUpdaterOptimizer.java
@@ -135,7 +135,7 @@
changed = true;
}
} else if (invokedMethod.isIdenticalTo(dexItemFactory.atomicReferenceUpdaterMethods.get)) {
- if (visitGet(context, invoke)) {
+ if (visitGet(context, invoke, dexItemFactory.sunMiscUnsafeMethods.getObjectVolatile)) {
changed = true;
}
} else if (invokedMethod.isIdenticalTo(dexItemFactory.atomicReferenceUpdaterMethods.set)) {
@@ -153,6 +153,10 @@
context, invoke, dexItemFactory.sunMiscUnsafeMethods.compareAndSwapInt)) {
changed = true;
}
+ } else if (invokedMethod.isIdenticalTo(dexItemFactory.atomicIntUpdaterMethods.get)) {
+ if (visitGet(context, invoke, dexItemFactory.sunMiscUnsafeMethods.getIntVolatile)) {
+ changed = true;
+ }
} else {
reportInfo(appView, new Event.CannotOptimize(invoke), Reason.NOT_SUPPORTED);
}
@@ -190,7 +194,7 @@
}
/** Returns true if {@code invoke} was rewritten. */
- private boolean visitGet(OptimizationContext context, InvokeVirtual invoke) {
+ private boolean visitGet(OptimizationContext context, InvokeVirtual invoke, DexMethod target) {
var resolvedUpdater = resolveUpdater(context, invoke);
if (resolvedUpdater == null) {
return false;
@@ -205,7 +209,7 @@
reportInfo(
appView,
new Event.CanOptimize(invoke, resolvedUpdater.isNullable, resolvedHolder.isNullable));
- rewriteGet(context, invoke, resolvedUpdater, resolvedHolder);
+ rewriteGet(context, invoke, resolvedUpdater, resolvedHolder, target);
return true;
}
@@ -388,14 +392,15 @@
}
/**
- * Rewrites {@code updater.get(holder)} into {@code
- * SyntheticUnsafeClass.unsafe.getReferenceVolatile(holder, this.offset)}.
+ * Rewrites {@code updater.get(holder)} into {@code SyntheticUnsafeClass.unsafe.<target>(holder,
+ * this.offset)}.
*/
private void rewriteGet(
OptimizationContext context,
InvokeVirtual invoke,
ResolvedUpdater resolvedUpdater,
- ResolvedHolder resolvedHolder) {
+ ResolvedHolder resolvedHolder,
+ DexMethod target) {
var position = invoke.getPosition();
var instructions = new ArrayList<Instruction>(4);
@@ -419,15 +424,9 @@
insertInstructionsBeforeCurrentInstruction(context.it, instructions);
// Call underlying unsafe method.
- DexMethod unsafeGetMethod =
- dexItemFactory.createMethod(
- dexItemFactory.sunMiscUnsafeType,
- dexItemFactory.createProto(
- dexItemFactory.objectType, dexItemFactory.objectType, dexItemFactory.longType),
- "getObjectVolatile");
Instruction unsafeGet =
new InvokeVirtual(
- unsafeGetMethod,
+ target,
invoke.outValue(),
ImmutableList.of(unsafeInstance.outValue(), resolvedHolder.value, offset.outValue()));
unsafeGet.setPosition(position);
diff --git a/src/test/java/com/android/tools/r8/optimize/atomicfieldupdater/integer/AtomicFieldUpdaterGetTest.java b/src/test/java/com/android/tools/r8/optimize/atomicfieldupdater/integer/AtomicFieldUpdaterGetTest.java
index ce1248d..0821557 100644
--- a/src/test/java/com/android/tools/r8/optimize/atomicfieldupdater/integer/AtomicFieldUpdaterGetTest.java
+++ b/src/test/java/com/android/tools/r8/optimize/atomicfieldupdater/integer/AtomicFieldUpdaterGetTest.java
@@ -43,16 +43,29 @@
diagnostics -> {
diagnostics.assertInfosMatch(
diagnosticMessage(containsString("Can instrument")),
- diagnosticMessage(containsString("Cannot optimize")),
- diagnosticMessage(containsString("Cannot remove")));
+ diagnosticMessage(containsString("Can optimize"))
+ // TODO(b/453628974): The field should be removed once nullability analysis is
+ // more precise.
+ );
})
.inspect(
inspector -> {
MethodSubject method = inspector.clazz(testClass).mainMethod();
- assertThat(
- method,
- CodeMatchers.invokesMethodWithHolderAndName(
- AtomicIntegerFieldUpdater.class, "get"));
+ if (isOptimizationOn()) {
+ assertThat(
+ method,
+ CodeMatchers.invokesMethod(
+ inspector
+ .getFactory()
+ .sunMiscUnsafeMethods
+ .getIntVolatile
+ .asMethodReference()));
+ } else {
+ assertThat(
+ method,
+ CodeMatchers.invokesMethodWithHolderAndName(
+ AtomicIntegerFieldUpdater.class, "get"));
+ }
})
.run(parameters.getRuntime(), testClass)
.assertSuccessWithOutputLines("123");
diff --git a/src/test/java/com/android/tools/r8/optimize/atomicfieldupdater/reference/AtomicFieldUpdaterGetTest.java b/src/test/java/com/android/tools/r8/optimize/atomicfieldupdater/reference/AtomicFieldUpdaterGetTest.java
index b437870..d65f072 100644
--- a/src/test/java/com/android/tools/r8/optimize/atomicfieldupdater/reference/AtomicFieldUpdaterGetTest.java
+++ b/src/test/java/com/android/tools/r8/optimize/atomicfieldupdater/reference/AtomicFieldUpdaterGetTest.java
@@ -55,8 +55,12 @@
if (isOptimizationOn()) {
assertThat(
method,
- CodeMatchers.invokesMethodWithHolderAndName(
- "sun.misc.Unsafe", "getObjectVolatile"));
+ CodeMatchers.invokesMethod(
+ inspector
+ .getFactory()
+ .sunMiscUnsafeMethods
+ .getObjectVolatile
+ .asMethodReference()));
} else {
assertThat(
method,
diff --git a/src/test/java/com/android/tools/r8/optimize/atomicfieldupdater/reference/AtomicFieldUpdaterNullableHolderTest.java b/src/test/java/com/android/tools/r8/optimize/atomicfieldupdater/reference/AtomicFieldUpdaterNullableHolderTest.java
index 7b368a4..3713b7b 100644
--- a/src/test/java/com/android/tools/r8/optimize/atomicfieldupdater/reference/AtomicFieldUpdaterNullableHolderTest.java
+++ b/src/test/java/com/android/tools/r8/optimize/atomicfieldupdater/reference/AtomicFieldUpdaterNullableHolderTest.java
@@ -54,8 +54,12 @@
if (isOptimizationOn()) {
assertThat(
method,
- CodeMatchers.invokesMethodWithHolderAndName(
- "sun.misc.Unsafe", "getObjectVolatile"));
+ CodeMatchers.invokesMethod(
+ inspector
+ .getFactory()
+ .sunMiscUnsafeMethods
+ .getObjectVolatile
+ .asMethodReference()));
} else {
assertThat(
method,
