[llvm-branch-commits] [llvm] [AMDGPU] Preserve noclobber after acquire operations from a different addrspace (PR #231855)

[Fabian Ritter via llvm-branch-commits]

================
@@ -411,15 +411,25 @@ bool isReallyAClobber(const Value *Ptr, MemoryDef *Def, AAResults *AA) {
     return !AA->isNoAlias(I->getPointerOperand(), Ptr);
   };
 
-  if (const auto *F = dyn_cast<FenceInst>(DefInst))
-    return isAcquireOrStronger(F->getOrdering());
+  if (const auto *F = dyn_cast<FenceInst>(DefInst)) {
+    if (!isAcquireOrStronger(F->getOrdering()))
+      return false;
+    SIAtomicAddrSpace AccessedAS =
+        toSIAtomicAddrSpace(Ptr->getType()->getPointerAddressSpace());
+    std::optional<SIAtomicAddrSpace> SynchronizedAS =
+        getSynchronizeAddrSpace(*F);
+    // If SynchronizedAS is std::nullopt, it refers to all address spaces.
+    return !SynchronizedAS ||
+           (*SynchronizedAS & AccessedAS) != SIAtomicAddrSpace::NONE;
+  }
 
   if (const auto *I = dyn_cast<AtomicRMWInst>(DefInst))
     return isAcquireOrStronger(I->getOrdering()) || MayAlias(I);
 
   if (const auto *I = dyn_cast<AtomicCmpXchgInst>(DefInst))
     return isAcquireOrStronger(I->getMergedOrdering()) || MayAlias(I);
 
+  // TODO This is pessimistic for one-as load-acquires from a different AS.
----------------
ritter-x2a wrote:

Because the logic for determining if a syncscope is `one-as` is currently also quite entangled with the backend (even more than that for interpreting the `amdgpu-synchronize-as` MMRA). I think we'll need some new module-level state that we pipe through the pass pipeline (or re-create when necessary).
There was also discussion whether `one-as` syncscopes should be removed in favor of MMRAs or something similar, so I'm not sure if implementing this optimization is worth the investment.

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