JarSourceCode: Don't generate method synchronization for CF output
Since the DEX format does not have synchronized methods, R8 must output
monitor enter/exit instructions at beginning/end of synchronized
methods. However, this is not needed for the CF format since the JVM
automatically synchronizes when calling or returning from a synchronized
method.
Bug: 73921688
Change-Id: Id77030a3ac1e06dc811b2922a881e063221acba9
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
index 12429e9..aa51da1 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
@@ -218,8 +218,16 @@
return (node.access & Opcodes.ACC_STATIC) > 0;
}
- private boolean isSynchronized() {
- return (node.access & Opcodes.ACC_SYNCHRONIZED) > 0;
+ /**
+ * Determine if we should emit monitor enter/exit instructions at method entry/exit.
+ *
+ * @return true if we are generating Dex and method is marked synchronized, otherwise false.
+ */
+ private boolean generateMethodSynchronization() {
+ // When generating class files, don't treat the method specially because it is synchronized.
+ // At runtime, the JVM will automatically perform the correct monitor enter/exit instructions.
+ return !application.options.isGeneratingClassFiles()
+ && (node.access & Opcodes.ACC_SYNCHRONIZED) > 0;
}
private int formalParameterCount() {
@@ -348,7 +356,7 @@
}
}
- if (isSynchronized()) {
+ if (generateMethodSynchronization()) {
generatingMethodSynchronization = true;
Type clazzType = application.getAsmType(clazz.toDescriptorString());
int monitorRegister;
@@ -450,7 +458,7 @@
@Override
public void buildPostlude(IRBuilder builder) {
- if (isSynchronized()) {
+ if (generateMethodSynchronization()) {
generatingMethodSynchronization = true;
buildMonitorExit(builder);
generatingMethodSynchronization = false;
@@ -458,7 +466,7 @@
}
private void buildExceptionalPostlude(IRBuilder builder) {
- assert isSynchronized();
+ assert generateMethodSynchronization();
generatingMethodSynchronization = true;
currentPosition = getExceptionalExitPosition();
buildMonitorExit(builder);
@@ -748,7 +756,7 @@
handlers.add(tryCatchBlock);
}
}
- if (isSynchronized()) {
+ if (generateMethodSynchronization()) {
// Add synchronized exceptional exit for synchronized-method instructions without a default.
assert handlers.isEmpty() || handlers.get(handlers.size() - 1).getType() != null;
handlers.add(EXCEPTIONAL_SYNC_EXIT);
@@ -2296,7 +2304,7 @@
if (handlers.isEmpty()) {
return true;
}
- if (!isSynchronized() || handlers.size() > 1) {
+ if (!generateMethodSynchronization() || handlers.size() > 1) {
return false;
}
return handlers.get(0) == EXCEPTIONAL_SYNC_EXIT;
diff --git a/src/test/java/com/android/tools/r8/cf/SynchronizedNoopTestRunner.java b/src/test/java/com/android/tools/r8/cf/SynchronizedNoopTestRunner.java
index 20acb9f..7565dee 100644
--- a/src/test/java/com/android/tools/r8/cf/SynchronizedNoopTestRunner.java
+++ b/src/test/java/com/android/tools/r8/cf/SynchronizedNoopTestRunner.java
@@ -6,6 +6,8 @@
package com.android.tools.r8.cf;
+import static org.junit.Assert.assertFalse;
+
import com.android.tools.r8.R8;
import com.android.tools.r8.R8Command;
import com.android.tools.r8.ToolHelper;
@@ -53,7 +55,6 @@
insn ->
insn.getOpcode() == Opcodes.MONITORENTER
|| insn.getOpcode() == Opcodes.MONITOREXIT);
- // TODO(b/73921688): Should not have monitor instruction here
- assert hasMonitor;
+ assertFalse(hasMonitor);
}
}