[PATCH] D136525: [M68k] Add codegen pattern for atomic load / store

[Min-Yih Hsu via Phabricator via llvm-commits]

myhsu added a comment.

I think it makes sense to assume m68k always runs on uniprocessors. GCC / libgcc and m68k port of Linux also makes the same assumption. For instance, gcc simply lowers atomic fence into `asm volatile("" ::: "memory")` (meaning the only thing we need to do is preventing compiler from reordering across the fence).

Though I don't think the changes on `getMaxAtomicSizeInBitsSupported` can really be justified. Given the fact that you can lower unsupported atomic operations to library calls in legalizer as well.



================
Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:176
   return Alignment >= Size &&
-         Size <= TLI->getMaxAtomicSizeInBitsSupported() / 8;
+         Size <= TLI->getMaxAtomicSizeInBitsSupported(I) / 8;
 }
----------------
I think you tried to turn every atomic operations but atomic_load / store / cmpxchg into libcall here. But even we don't turn them into libcalls in this pass, we still can do that during legalization by marking the corresponding SDNode as Expand or LibCall, right?


================
Comment at: llvm/lib/Target/M68k/M68kInstrAtomics.td:27
+
+multiclass AtomicPatterns {
+  foreach size = [8, 16, 32] in {
----------------
Why do we need to wrap these patterns with multiclass?


================
Comment at: llvm/test/MC/Disassembler/M68k/atomics.txt:3
+
+# CHECK: cas.b %d3, %d2, (%a2)
+0x0a 0xd2 0x00 0x83
----------------
what about other sizes?


================
Comment at: llvm/test/MC/M68k/Atomics/cas.s:5
+; CHECK-SAME: ; encoding: [0x0a,0xd2,0x00,0x83]
+cas.b %d3, %d2, (%a2)
----------------
ditto other sizes


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136525/new/

https://reviews.llvm.org/D136525