[AA] Improve precision for monotonic atomic load/store operations

[jinhuang1102]

Refines getModRefInfo for Monotonic and Unordered atomic operations to perform a full alias evaluation instead of conservatively returning ModRef.

This allows AA to return more precise results (e.g., NoModRef), unblocking optimizations in passes like DSE/PRE when the pointers are proven not to alias.

LIT testing:

1. Fixed the XFAIL test @test9 in DSE/atomic-todo.ll, where the improved AA now allows DSE to correctly remove a dead store.
2. Added test @test9a to the same file to ensure DSE is still blocked when pointers may alias.
3. I am also reviewing tests in GVN/PRE to see where this change may be beneficial. While all existing tests pass, I will update this PR with any new GVN test cases in a follow-up commit.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-analysis

Author: Jin Huang (jinhuang1102)

Changes

Refines getModRefInfo for Monotonic and Unordered atomic operations to perform a full alias evaluation instead of conservatively returning ModRef.

This allows AA to return more precise results (e.g., NoModRef), unblocking optimizations in passes like DSE/PRE when the pointers are proven not to alias.

LIT testing:

1. Fixed the XFAIL test @<!-- -->test9 in DSE/atomic-todo.ll, where the improved AA now allows DSE to correctly remove a dead store.
2. Added test @<!-- -->test9a to the same file to ensure DSE is still blocked when pointers may alias.
3. I am also reviewing tests in GVN/PRE to see where this change may be beneficial. While all existing tests pass, I will update this PR with any new GVN test cases in a follow-up commit.

---

Full diff: https://github.com/llvm/llvm-project/pull/158169.diff

2 Files Affected:

• (modified) llvm/lib/Analysis/AliasAnalysis.cpp (+12-3)
• (modified) llvm/test/Transforms/DeadStoreElimination/atomic-todo.ll (+15-2)

diff --git a/llvm/lib/Analysis/AliasAnalysis.cpp b/llvm/lib/Analysis/AliasAnalysis.cpp
index 3ec009ca4adde..cb9577aac405a 100644
--- a/llvm/lib/Analysis/AliasAnalysis.cpp
+++ b/llvm/lib/Analysis/AliasAnalysis.cpp
@@ -421,9 +421,18 @@ ModRefInfo AAResults::getModRefInfo(const LoadInst *L,
                                     const MemoryLocation &Loc,
                                     AAQueryInfo &AAQI) {
   // Be conservative in the face of atomic.
-  if (isStrongerThan(L->getOrdering(), AtomicOrdering::Unordered))
+  if (isStrongerThan(L->getOrdering(), AtomicOrdering::Monotonic))
     return ModRefInfo::ModRef;
 
+  // For Monotonic and unordered atomic loads, if the locations are not NoAlias,
+  // we must be conservative and return ModRef to prevent unsafe reordering of
+  // accesses to the same memory.
+  if (L->isAtomic()){
+    if (Loc.Ptr &&
+        alias(MemoryLocation::get(L), Loc, AAQI, L) != AliasResult::NoAlias)
+      return ModRefInfo::ModRef;
+  }
+
   // If the load address doesn't alias the given address, it doesn't read
   // or write the specified memory.
   if (Loc.Ptr) {
@@ -439,7 +448,7 @@ ModRefInfo AAResults::getModRefInfo(const StoreInst *S,
                                     const MemoryLocation &Loc,
                                     AAQueryInfo &AAQI) {
   // Be conservative in the face of atomic.
-  if (isStrongerThan(S->getOrdering(), AtomicOrdering::Unordered))
+  if (isStrongerThan(S->getOrdering(), AtomicOrdering::Monotonic))
     return ModRefInfo::ModRef;
 
   if (Loc.Ptr) {
@@ -458,7 +467,7 @@ ModRefInfo AAResults::getModRefInfo(const StoreInst *S,
   }
 
   // Otherwise, a store just writes.
-  return ModRefInfo::Mod;
+  return ModRefInfo::ModRef;
 }
 
 ModRefInfo AAResults::getModRefInfo(const FenceInst *S,
diff --git a/llvm/test/Transforms/DeadStoreElimination/atomic-todo.ll b/llvm/test/Transforms/DeadStoreElimination/atomic-todo.ll
index 1c160442f8579..85a4a7fdd79e3 100644
--- a/llvm/test/Transforms/DeadStoreElimination/atomic-todo.ll
+++ b/llvm/test/Transforms/DeadStoreElimination/atomic-todo.ll
@@ -1,4 +1,3 @@
-; XFAIL: *
 ; RUN: opt -passes=dse -S < %s | FileCheck %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
@@ -11,7 +10,7 @@ target triple = "x86_64-apple-macosx10.7.0"
 @x = common global i32 0, align 4
 @y = common global i32 0, align 4
 
-; DSE across monotonic load (allowed as long as the eliminated store isUnordered)
+; DSE across monotonic load (allowed if the monotonic load's address is NoAlias)
 define i32 @test9() {
 ; CHECK-LABEL: test9
 ; CHECK-NOT: store i32 0
@@ -21,3 +20,17 @@ define i32 @test9() {
   store i32 1, ptr @x
   ret i32 %x
 }
+
+; DSE across monotonic load (blocked if the atomic load's address isn't NoAlias)
+define i32 @test9a(ptr %ptr) {
+; CHECK-LABEL: @test9a(
+; CHECK-NEXT:    store i32 0, ptr @x, align 4
+; CHECK-NEXT:    [[X:%.*]] = load atomic i32, ptr [[PTR:%.*]] monotonic, align 4
+; CHECK-NEXT:    store i32 1, ptr @x, align 4
+; CHECK-NEXT:    ret i32 [[X]]
+;
+  store i32 0, ptr @x
+  %x = load atomic i32, ptr %ptr monotonic, align 4
+  store i32 1, ptr @x
+  ret i32 %x
+}

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[nikic]

This would be too much for unordered atomics.

This also duplicates the code block below. I'd drop this block and then insert a check before the existing return that does:

if (isStrongerThanUnordered(L->getOrdering()))
  return ModRefInfo::ModRef;

[jinhuang1102]

I see. You means that treat unordered like the non-atomic load and add the following code:

if (isStrongerThanUnordered(L->getOrdering())) 
  return ModRefInfo::ModRef;

After the AliasResult AR check:

  if (Loc.Ptr) {
    AliasResult AR = alias(MemoryLocation::get(L), Loc, AAQI, L);
    if (AR == AliasResult::NoAlias)
      return ModRefInfo::NoModRef;
  }

Am I right?

[nikic]

Similar as in the load case, should only return ModRef if stronger than unordered.

[jinhuang1102]

Do you think we need to add the similar if condition here?

[jinhuang1102]

I see. You means that treat unordered like the non-atomic load and add the following code:

if (isStrongerThanUnordered(L->getOrdering())) 
  return ModRefInfo::ModRef;

After the AliasResult AR check:

  if (Loc.Ptr) {
    AliasResult AR = alias(MemoryLocation::get(L), Loc, AAQI, L);
    if (AR == AliasResult::NoAlias)
      return ModRefInfo::NoModRef;
  }

Am I right?

[jinhuang1102]

Do you think we need to add the similar if condition here?

[david-xl]

Change the comments to assertion instead. The comment is not helpful.

[jinhuang1102]

Replace the comments by

assert(!isStrongerThanMonotonic(L->getOrdering()) &&
            "Stronger atomic orderings should have been handled above!" );

[jinhuang1102]

move the @test9 from atmonic-todo.ll to atomic.ll and rename the test cases.

[david-xl]

lgtm, but please wait for nikic@'s approval.