[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:
> 0x59616e wrote:
> > 0x59616e wrote:
> > > 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 ?
> > Here is where the undesirable expansion happen : 
> > 
> > https://github.com/llvm/llvm-project/blob/69d117edc29d4c74e034d8474433e981b2702898/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp#L5109
> *Marking it as `LibCall` CANNOT*
We may want to set the `ATOMIC_CMP_SWAP` as `LibCall` when the target is below 020. But that also turns into `__sync_*` library call.

Here is my workaround: You can see from here:

https://github.com/llvm/llvm-project/blob/main/llvm/lib/CodeGen/AtomicExpandPass.cpp#L178

that the AtomicExpandPass will check the alignment to decide whether should give it a go. We can add a IR pass that change the alignment of `cmpxchg` to zero so that the AtomicExpandPass will work for us.

Do you think this is auspicious ?




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

https://reviews.llvm.org/D136525