Ignore monitor exit for field load elimination
This makes the code be more "precise", but does not change
the output, due to the way field values are currently propagated
to successors.
Bug: 111250398, 111380066
Change-Id: I2444ddc17af82274166744a8c9a8700a50877d40
diff --git a/src/main/java/com/android/tools/r8/ir/code/Monitor.java b/src/main/java/com/android/tools/r8/ir/code/Monitor.java
index 6b3c89c..3b1b2d1 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Monitor.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Monitor.java
@@ -34,6 +34,14 @@
return inValues.get(0);
}
+ public boolean isEnter() {
+ return type == Type.ENTER;
+ }
+
+ public boolean isExit() {
+ return type == Type.EXIT;
+ }
+
@Override
public void buildDex(DexBuilder builder) {
// If the monitor object is an argument, we use the argument register for all the monitor
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java b/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java
index dee935e..905d4a8 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java
@@ -132,7 +132,8 @@
}
}
}
- if (instruction.isMonitor() || instruction.isInvokeMethod()) {
+ if ((instruction.isMonitor() && instruction.asMonitor().isEnter())
+ || instruction.isInvokeMethod()) {
killAllActiveFields();
}
}
diff --git a/src/test/java/com/android/tools/r8/regress/b111250398/B111250398.java b/src/test/java/com/android/tools/r8/regress/b111250398/B111250398.java
index 90fba5f..84fc5c4 100644
--- a/src/test/java/com/android/tools/r8/regress/b111250398/B111250398.java
+++ b/src/test/java/com/android/tools/r8/regress/b111250398/B111250398.java
@@ -91,6 +91,16 @@
t = f;
}
+ public void mfWithMonitor() {
+ t = f;
+ synchronized (this) {
+ t = f;
+ t = f;
+ }
+ t = f;
+ t = f;
+ }
+
public void msf() {
t = sf;
t = sf;
@@ -231,6 +241,8 @@
assertThat(classA, isPresent());
MethodSubject mfOnA = classA.method("void", "mf", ImmutableList.of());
assertThat(mfOnA, isPresent());
+ MethodSubject mfWithMonitorOnA = classA.method("void", "mfWithMonitor", ImmutableList.of());
+ assertThat(mfWithMonitorOnA, isPresent());
MethodSubject msfOnA = classA.method("void", "msf", ImmutableList.of());
assertThat(msfOnA, isPresent());
MethodSubject mvOnA = classA.method("void", "mv", ImmutableList.of());
@@ -264,6 +276,13 @@
// compilation (R8) will eliminate field loads on non-volatile fields.
assertEquals(1, countIget(mfOnA.getMethod().getCode().asDexCode(), fOnA.getField().field));
assertEquals(1, countSget(msfOnA.getMethod().getCode().asDexCode(), sfOnA.getField().field));
+ // TODO(111380066). This could be 2 in stead of 4, but right now the optimization tracks the
+ // combined set of fields for all successors, and for synchronized code all blocks have
+ // exceptional edges for ensuring monitor exit causing the active load to be invalidated for
+ // both normal and exceptional successors.
+ assertEquals(4,
+ countIget(mfWithMonitorOnA.getMethod().getCode().asDexCode(), fOnA.getField().field));
+
// For fields on other class both separate compilation (D8) and whole program
// compilation (R8) will differ in the eliminated field loads of non-volatile fields.
assertEquals(mfOnBGets,
