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

[Sheng via Phabricator via llvm-commits]

0x59616e added inline comments.


================
Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:176
   return Alignment >= Size &&
-         Size <= TLI->getMaxAtomicSizeInBitsSupported() / 8;
+         Size <= TLI->getMaxAtomicSizeInBitsSupported(I) / 8;
 }
----------------
0x59616e wrote:
> myhsu wrote:
> > myhsu wrote:
> > > 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?
> > *corresponding operation as Expand or LibCall
> I'll look into it.
Marking it as `LibCall` can stop the type legalizer from expanding the AtomicStore with 64bits operand to AtomicSwap, which will be transformed to `__sync_lock_test_and_set` --- a function that I can't find in `libatomic.a`.

We may need to find a way to stop the type legalizer from doing it so that the legalizer can expand AtomicStore to library call. Any ideas ?


================
Comment at: llvm/lib/Target/M68k/M68kInstrAtomics.td:27
+
+multiclass AtomicPatterns {
+  foreach size = [8, 16, 32] in {
----------------
RKSimon wrote:
> 0x59616e wrote:
> > myhsu wrote:
> > > Why do we need to wrap these patterns with multiclass?
> > Yeah that's verbose. I didn't think this through.
> Probably the best way to make this 020+ only is to wrap the patterns, something like:
> ```
> let Predicates = [FeatureISA20] {
>  foreach size = [8, 16, 32] in {
>   ....
>  }
> }
> ```
Thanks. I'll look into it.


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

https://reviews.llvm.org/D136525