[llvm] [LoopFusion] Reject loops containing atomic accesses (PR #201525)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Madhur Amilkanthwar (madhur13490)

<details>
<summary>Changes</summary>

The fusion legality check relies on DependenceInfo, which models memory dependences purely by address and does not capture the ordering or synchronization (synchronizes-with / fence) semantics of atomic operations. Fusing two loops interleaves their bodies, which can reorder atomics in ways that violate the memory model even when they touch different locations.

Mirror the existing volatile handling and invalidate any fusion candidate whose body contains an atomic access, adding a ContainsAtomicAccess statistic.

This is conservative: Instruction::isAtomic() is true for unordered and higher, so even unordered atomics (which carry no cross-thread ordering) are rejected. This matches the all-or-nothing volatile precedent and keeps the check simple; it can be narrowed to allow unordered later if needed.

Fixes #<!-- -->193770.

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


2 Files Affected:

- (modified) llvm/lib/Transforms/Scalar/LoopFuse.cpp (+12-12) 
- (added) llvm/test/Transforms/LoopFusion/cannot_fuse_atomic.ll (+39) 


``````````diff
diff --git a/llvm/lib/Transforms/Scalar/LoopFuse.cpp b/llvm/lib/Transforms/Scalar/LoopFuse.cpp
index bce9a04ddd0b2..9eeb3bea3a48d 100644
--- a/llvm/lib/Transforms/Scalar/LoopFuse.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopFuse.cpp
@@ -75,6 +75,7 @@ STATISTIC(InvalidLoopStructure, "Loop has invalid structure");
 STATISTIC(AddressTakenBB, "Basic block has address taken");
 STATISTIC(MayThrowException, "Loop may throw an exception");
 STATISTIC(ContainsVolatileAccess, "Loop contains a volatile access");
+STATISTIC(ContainsAtomicAccess, "Loop contains an atomic access");
 STATISTIC(NotSimplifiedForm, "Loop is not in simplified form");
 STATISTIC(InvalidDependencies, "Dependencies prevent fusion");
 STATISTIC(UnknownTripCount, "Loop has unknown trip count");
@@ -181,19 +182,18 @@ struct FusionCandidate {
           reportInvalidCandidate(MayThrowException);
           return;
         }
-        if (StoreInst *SI = dyn_cast<StoreInst>(&I)) {
-          if (SI->isVolatile()) {
-            invalidate();
-            reportInvalidCandidate(ContainsVolatileAccess);
-            return;
-          }
+        if (I.isVolatile()) {
+          invalidate();
+          reportInvalidCandidate(ContainsVolatileAccess);
+          return;
         }
-        if (LoadInst *LI = dyn_cast<LoadInst>(&I)) {
-          if (LI->isVolatile()) {
-            invalidate();
-            reportInvalidCandidate(ContainsVolatileAccess);
-            return;
-          }
+        // Atomic accesses impose ordering/synchronization constraints that the
+        // dependence analysis used for fusion does not model, so reordering
+        // them across the fused body could be unsafe.
+        if (I.isAtomic()) {
+          invalidate();
+          reportInvalidCandidate(ContainsAtomicAccess);
+          return;
         }
         if (I.mayWriteToMemory())
           MemWrites.push_back(&I);
diff --git a/llvm/test/Transforms/LoopFusion/cannot_fuse_atomic.ll b/llvm/test/Transforms/LoopFusion/cannot_fuse_atomic.ll
new file mode 100644
index 0000000000000..f511d3fc85562
--- /dev/null
+++ b/llvm/test/Transforms/LoopFusion/cannot_fuse_atomic.ll
@@ -0,0 +1,39 @@
+; RUN: opt -S -passes=loop-simplify,loop-fusion \
+; RUN:   -pass-remarks-analysis=loop-fusion -disable-output < %s 2>&1 \
+; RUN:   | FileCheck %s
+; REQUIRES: asserts
+
+; Atomic accesses impose ordering constraints that the dependence analysis
+; used by fusion does not model, so a loop containing one is not a fusion
+; candidate.
+
+; CHECK: [atomic_access]: Loop is not a candidate for fusion: Loop contains an atomic access
+
+define void @atomic_access(ptr noalias %arg) {
+entry:
+  br label %for.body1
+
+for.body1:                                        ; preds = %entry, %for.body1
+  %i1 = phi i64 [ 0, %entry ], [ %inc1, %for.body1 ]
+  %gep1 = getelementptr inbounds i32, ptr %arg, i64 %i1
+  %v1 = trunc i64 %i1 to i32
+  store atomic i32 %v1, ptr %gep1 monotonic, align 4
+  %inc1 = add nuw nsw i64 %i1, 1
+  %cond1 = icmp ne i64 %inc1, 100
+  br i1 %cond1, label %for.body1, label %for.body2.preheader
+
+for.body2.preheader:                              ; preds = %for.body1
+  br label %for.body2
+
+for.body2:                                        ; preds = %for.body2.preheader, %for.body2
+  %i2 = phi i64 [ 0, %for.body2.preheader ], [ %inc2, %for.body2 ]
+  %gep2 = getelementptr inbounds i32, ptr %arg, i64 %i2
+  %v2 = trunc i64 %i2 to i32
+  store i32 %v2, ptr %gep2, align 4
+  %inc2 = add nuw nsw i64 %i2, 1
+  %cond2 = icmp ne i64 %inc2, 100
+  br i1 %cond2, label %for.body2, label %exit
+
+exit:                                             ; preds = %for.body2
+  ret void
+}

``````````

</details>


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