[llvm] [IR][AtomicExpand] Add elementwise modifier to atomicrmw; automatically expand for all targets (PR #189517)

[Antonio Frighetto via llvm-commits]

================
@@ -265,6 +268,23 @@ static bool atomicSizeSupported(const TargetLowering *TLI, Inst *I) {
          Size <= TLI->getMaxAtomicSizeInBitsSupported() / 8;
 }
 
+/// Returns true if we can lower atomicrmw elementwise using normal atomicrmw.
+bool AtomicExpandImpl::canReuseWholeValueAtomicRMW(AtomicRMWInst *AI) {
+  assert(AI->isElementwise() && "expected elementwise atomicrmw");
+
+  // Integer non-elementwise vector atomicrmw is illegal IR, so we need to be
+  // careful to reject these before removing the elementwise modifier.
+  if (!AI->isFloatingPointOperation())
+    return false;
+
+  AI->setElementwise(false);
----------------
antoniofrighetto wrote:

> I actually had a shouldExpandAtomicRMWElementwiseInIR in my first draft where the original shouldExpandAtomicRMWInIR wouldn't handle the elementwise case.

Thanks, I was initially indeed a bit perplexed not to see a similar method. I can understand why this may have been pushed back, but even so, for the time being, I think this is orthogonal to the current PR, and can be addressed in a follow-up PR when introducing the NVPTX changes.

> Because AtomicExpandPass calls both versions. So for X86 which needs access to the instruction, it'd be weird for them to have to implement the first version.

Okay, I think I see the point now. When I suggested having a stateless `shouldExpandAtomicRMWInIR(AtomicRMWInst::BinOp Op, ...)` overload with the above implementation, I thought it would have been fine for the purpose of `canReuseWholeValueAtomicRMW`, which is _always_ consulted with `elementwise`, and this routine would have been the only user of the stateless overload.

IIUC, since X86 and friends do not support elementwise vector atomicrmw, you wouldn't really have had to override the stateless method for X86, and instead, just default to the virtual method implementation above (X86 would just simply continue to override the `shouldExpandAtomicRMWInIR(AtomicRMWInst *AI)`, no need for additional changes as X86 would never query canReuseWholeValueAtomicRMW). This should be fine for the majority of the targets, though not for AMDGPU and, clearly, NVTPX; so the latter ones would need to override the stateless `shouldExpandAtomicRMWInIR` method. Whether this is fine or not should boil down to asking whether X86 et alia would ever want to implement `elementwise atomicrmw`, which I don't believe it should be the case? Please let me know if I'm missing something.

https://github.com/llvm/llvm-project/pull/189517