[PATCH] D103614: [PowerPC][AIX][RFC] Generate inlined quadword lock free atomic operations via AtomicExpand

Tue Jun 22 21:00:57 PDT 2021

lkail added inline comments.

================
Comment at: llvm/include/llvm/IR/IntrinsicsPowerPC.td:1545
+  def int_ppc_atomicrmw_umax_i128 : AtomicRMW128Intrinsic;
+  def int_ppc_atomicrmw_umin_i128 : AtomicRMW128Intrinsic;
+}
----------------
efriedma wrote:
> Adding IR intrinsics for this is a little weird.  Is there any reason you can't just use SelectionDAG custom legalization for these operations?  I mean, not that it doesn't work this way, but it seems more complicated overall.
Exploit AtomicExpandPass looks easier from my side :). And what's more current PPC backend also has spilling issues with `-O0`(fastregalloc enabled), see https://bugs.llvm.org/show_bug.cgi?id=50780. Maybe we should unify PPC's instruction selection of atomic operations one day (PPC also has partword atomics which is feasible to use MaskedIntrinsic) which I need more feedback from @nemanjai and @jsji .

================
Comment at: llvm/lib/Target/PowerPC/PPCInstr64Bit.td:318
+// Atomic pseudo instructions expanded post-ra.
+def ATOMIC_SWAP_I128 : AtomicRMW128<"#ATOMIC_SWAP_I128">;
+def ATOMIC_LOAD_ADD_I128 : AtomicRMW128<"#ATOMIC_LOAD_ADD_I128">;
----------------
efriedma wrote:
> Do you need to specify the size of these instructions somewhere, if you're expanding them after branch relaxation?
Good point. There is `PPCBranchSelector` pass serves as branch relaxation and PowerPC hasn't implemented LLVM MC's branch relaxation. I should have put the expansion before `PPCBranchSelector`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103614