Use optimised dmb on arm64 where possible on mini-mono runtime

- And fix up some FIXMEs & comments re cpobj and cpblk
dotnet · Sep 19, 2024 · 9cf3fd6 · 9cf3fd6
1 parent 9a103d1
commit 9cf3fd6
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 23 deletions.
diff --git a/src/mono/mono/mini/memory-access.c b/src/mono/mono/mini/memory-access.c
@@ -556,17 +556,9 @@ mini_emit_memory_copy_bytes (MonoCompile *cfg, MonoInst *dest, MonoInst *src, Mo
 {
 	int align = (ins_flag & MONO_INST_UNALIGNED) ? 1 : TARGET_SIZEOF_VOID_P;
 
-	/*
-	 * FIXME: It's unclear whether we should be emitting both the acquire
-	 * and release barriers for cpblk. It is technically both a load and
-	 * store operation, so it seems like that's the sensible thing to do.
-	 *
-	 * FIXME: We emit full barriers on both sides of the operation for
-	 * simplicity. We should have a separate atomic memcpy method instead.
-	 */
 	if (ins_flag & MONO_INST_VOLATILE) {
-		/* Volatile loads have acquire semantics, see 12.6.7 in Ecma 335 */
-		mini_emit_memory_barrier (cfg, MONO_MEMORY_BARRIER_SEQ);
+		/* Volatile stores have release semantics, see 12.6.7 in Ecma 335 */
+		mini_emit_memory_barrier (cfg, MONO_MEMORY_BARRIER_REL);
 	}
 
 	if ((cfg->opt & MONO_OPT_INTRINS) && (size->opcode == OP_ICONST)) {
@@ -577,7 +569,7 @@ mini_emit_memory_copy_bytes (MonoCompile *cfg, MonoInst *dest, MonoInst *src, Mo
 
 	if (ins_flag & MONO_INST_VOLATILE) {
 		/* Volatile loads have acquire semantics, see 12.6.7 in Ecma 335 */
-		mini_emit_memory_barrier (cfg, MONO_MEMORY_BARRIER_SEQ);
+		mini_emit_memory_barrier (cfg, MONO_MEMORY_BARRIER_ACQ);
 	}
 }
 
@@ -612,24 +604,16 @@ mini_emit_memory_copy (MonoCompile *cfg, MonoInst *dest, MonoInst *src, MonoClas
 	if (ins_flag & MONO_INST_UNALIGNED)
 		explicit_align = 1;
 
-	/*
-	 * FIXME: It's unclear whether we should be emitting both the acquire
-	 * and release barriers for cpblk. It is technically both a load and
-	 * store operation, so it seems like that's the sensible thing to do.
-	 *
-	 * FIXME: We emit full barriers on both sides of the operation for
-	 * simplicity. We should have a separate atomic memcpy method instead.
-	 */
 	if (ins_flag & MONO_INST_VOLATILE) {
-		/* Volatile loads have acquire semantics, see 12.6.7 in Ecma 335 */
-		mini_emit_memory_barrier (cfg, MONO_MEMORY_BARRIER_SEQ);
+		/* Volatile stores have release semantics, see 12.6.7 in Ecma 335 */
+		mini_emit_memory_barrier (cfg, MONO_MEMORY_BARRIER_REL);
 	}
 
 	mini_emit_memory_copy_internal (cfg, dest, src, klass, explicit_align, native, (ins_flag & MONO_INST_STACK_STORE) != 0);
 
 	if (ins_flag & MONO_INST_VOLATILE) {
 		/* Volatile loads have acquire semantics, see 12.6.7 in Ecma 335 */
-		mini_emit_memory_barrier (cfg, MONO_MEMORY_BARRIER_SEQ);
+		mini_emit_memory_barrier (cfg, MONO_MEMORY_BARRIER_ACQ);
 	}
 }
 #else /* !DISABLE_JIT */

diff --git a/src/mono/mono/mini/mini-arm64.c b/src/mono/mono/mini/mini-arm64.c
@@ -5058,7 +5058,10 @@ mono_arch_output_basic_block (MonoCompile *cfg, MonoBasicBlock *bb)
 			break;
 			/* Atomic */
 		case OP_MEMORY_BARRIER:
-			arm_dmb (code, ARM_DMB_ISH);
+			if (ins->backend.memory_barrier_kind == MONO_MEMORY_BARRIER_ACQ)
+				arm_dmb (code, ARM_DMB_ISHLD);
+			else
+				arm_dmb (code, ARM_DMB_ISH);
 			break;
 		case OP_ATOMIC_ADD_I4: {
 			guint8 *buf [16];