[llvm] [LoopFusion] Fix false-positive dependency blocking fusion of idempot… (PR #206401)

via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 15 21:10:22 PDT 2026


https://github.com/AntonyCJ30 updated https://github.com/llvm/llvm-project/pull/206401

>From 4f75c5896998b2061a67c40f412ecdbf0c1d18b0 Mon Sep 17 00:00:00 2001
From: AntonyCJ30 <cj6186609 at gmail@gmail.com>
Date: Mon, 29 Jun 2026 10:40:22 +0530
Subject: [PATCH] [LoopFusion] Fix false-positive dependency blocking fusion of
 idempotent stores

---
 llvm/lib/Transforms/Scalar/LoopFuse.cpp       | 325 +++++++-----------
 .../Transforms/LoopFusion/loop_invariant.ll   |  81 +++++
 2 files changed, 208 insertions(+), 198 deletions(-)

diff --git a/llvm/lib/Transforms/Scalar/LoopFuse.cpp b/llvm/lib/Transforms/Scalar/LoopFuse.cpp
index 1b83c971c01bf..79b44d5711aaa 100644
--- a/llvm/lib/Transforms/Scalar/LoopFuse.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopFuse.cpp
@@ -1113,6 +1113,14 @@ struct LoopFuser {
     auto DepResult = DI.depends(&I0, &I1);
     if (!DepResult)
       return true;
+    // If two stores write the same SSA value, fusion is safe regardless of
+    // aliasing — writing the same value twice is idempotent.
+    if (isa<StoreInst>(I0) && isa<StoreInst>(I1)) {
+      auto *S0 = cast<StoreInst>(&I0);
+      auto *S1 = cast<StoreInst>(&I1);
+      if (S0->getValueOperand() == S1->getValueOperand())
+        return true;
+    }
 #ifndef NDEBUG
     if (VerboseFusionDebugging) {
       LLVM_DEBUG(dbgs() << "DA res: "; DepResult->dump(dbgs());
@@ -1354,6 +1362,121 @@ struct LoopFuser {
     }
   }
 
+  /// Move FC1's header PHIs into FC0's header, insert the loop-carried PHIs
+  /// needed to keep SSA valid when FC0 exits without taking its back-edge, and
+  /// rewire both latches to form the fused loop. Latch dominator-tree updates
+  /// are appended to \p TreeUpdates for the caller to apply.
+  void rewireFusedHeaderPHIsAndLatches(
+      const FusionCandidate &FC0, const FusionCandidate &FC1,
+      const SmallVectorImpl<PHINode *> &OriginalFC0PHIs,
+      SmallVectorImpl<DominatorTree::UpdateType> &TreeUpdates) {
+    // Moves the phi nodes from the second to the first loops header block.
+    while (PHINode *PHI = dyn_cast<PHINode>(&FC1.Header->front())) {
+      if (SE.isSCEVable(PHI->getType()))
+        SE.forgetValue(PHI);
+      if (PHI->hasNUsesOrMore(1))
+        PHI->moveBefore(FC0.Header->getFirstInsertionPt());
+      else
+        PHI->eraseFromParent();
+    }
+
+    // Introduce new phi nodes in the second loop header to ensure
+    // exiting the first and jumping to the header of the second does not break
+    // the SSA property of the phis originally in the first loop. See also the
+    // comment above.
+    BasicBlock::iterator L1HeaderIP = FC1.Header->begin();
+    for (PHINode *LCPHI : OriginalFC0PHIs) {
+      int L1LatchBBIdx = LCPHI->getBasicBlockIndex(FC1.Latch);
+      assert(L1LatchBBIdx >= 0 &&
+             "Expected loop carried value to be rewired at this point!");
+
+      Value *LCV = LCPHI->getIncomingValue(L1LatchBBIdx);
+
+      PHINode *L1HeaderPHI =
+          PHINode::Create(LCV->getType(), 2, LCPHI->getName() + ".afterFC0");
+      L1HeaderPHI->insertBefore(L1HeaderIP);
+      L1HeaderPHI->addIncoming(LCV, FC0.Latch);
+      L1HeaderPHI->addIncoming(PoisonValue::get(LCV->getType()),
+                               FC0.ExitingBlock);
+
+      LCPHI->setIncomingValue(L1LatchBBIdx, L1HeaderPHI);
+    }
+
+    // Replace latch terminator destinations.
+    FC0.Latch->getTerminator()->replaceUsesOfWith(FC0.Header, FC1.Header);
+    FC1.Latch->getTerminator()->replaceUsesOfWith(FC1.Header, FC0.Header);
+
+    // Modify the latch branch of FC0 to be unconditional as both successors of
+    // the branch are the same.
+    simplifyLatchBranch(FC0);
+
+    // If FC0.Latch and FC0.ExitingBlock are the same then we have already
+    // performed the updates above.
+    if (FC0.Latch != FC0.ExitingBlock)
+      TreeUpdates.emplace_back(DominatorTree::UpdateType(
+          DominatorTree::Insert, FC0.Latch, FC1.Header));
+
+    TreeUpdates.emplace_back(DominatorTree::UpdateType(DominatorTree::Delete,
+                                                       FC0.Latch, FC0.Header));
+    TreeUpdates.emplace_back(DominatorTree::UpdateType(DominatorTree::Insert,
+                                                       FC1.Latch, FC0.Header));
+    TreeUpdates.emplace_back(DominatorTree::UpdateType(DominatorTree::Delete,
+                                                       FC1.Latch, FC1.Header));
+  }
+
+  /// Forget cached SCEV state for both loops, move all of FC1's blocks and
+  /// child loops into FC0, erase the now-empty FC1, and merge the latches.
+  /// Returns the fused loop (FC0.L).
+  Loop *finalizeFusedLoop(const FusionCandidate &FC0,
+                          const FusionCandidate &FC1) {
+    // Is there a way to keep SE up-to-date so we don't need to forget the loops
+    // and rebuild the information in subsequent passes of fusion?
+    // Note: Need to forget the loops before merging the loop latches, as
+    // mergeLatch may remove the only block in FC1.
+    SE.forgetLoop(FC1.L);
+    SE.forgetLoop(FC0.L);
+
+    // Merge the loops.
+    SmallVector<BasicBlock *, 8> Blocks(FC1.L->blocks());
+    for (BasicBlock *BB : Blocks) {
+      FC0.L->addBlockEntry(BB);
+      FC1.L->removeBlockFromLoop(BB);
+      if (LI.getLoopFor(BB) != FC1.L)
+        continue;
+      LI.changeLoopFor(BB, FC0.L);
+    }
+    while (!FC1.L->isInnermost()) {
+      const auto &ChildLoopIt = FC1.L->begin();
+      Loop *ChildLoop = *ChildLoopIt;
+      FC1.L->removeChildLoop(ChildLoopIt);
+      FC0.L->addChildLoop(ChildLoop);
+    }
+
+    // Delete the now empty loop L1.
+    LI.erase(FC1.L);
+
+    // Forget block dispositions as well, so that there are no dangling
+    // pointers to erased/free'ed blocks. It should be done after mergeLatch()
+    // since merging the latches may affect the dispositions.
+    SE.forgetBlockAndLoopDispositions();
+
+    // Move instructions from FC0.Latch to FC1.Latch.
+    // Note: mergeLatch requires an updated DT.
+    mergeLatch(FC0, FC1);
+
+#ifndef NDEBUG
+    assert(!verifyFunction(*FC0.Header->getParent(), &errs()));
+    assert(DT.verify(DominatorTree::VerificationLevel::Fast));
+    assert(PDT.verify());
+    LI.verify(DT);
+    SE.verify();
+#endif
+
+    LLVM_DEBUG(dbgs() << "Fusion done:\n");
+
+    return FC0.L;
+  }
+
   /// Fuse two fusion candidates, creating a new fused loop.
   ///
   /// This method contains the mechanics of fusing two loops, represented by \p
@@ -1472,58 +1595,7 @@ struct LoopFuser {
     TreeUpdates.emplace_back(DominatorTree::UpdateType(
         DominatorTree::Delete, FC1.Preheader, FC1.Header));
 
-    // Moves the phi nodes from the second to the first loops header block.
-    while (PHINode *PHI = dyn_cast<PHINode>(&FC1.Header->front())) {
-      if (SE.isSCEVable(PHI->getType()))
-        SE.forgetValue(PHI);
-      if (PHI->hasNUsesOrMore(1))
-        PHI->moveBefore(FC0.Header->getFirstInsertionPt());
-      else
-        PHI->eraseFromParent();
-    }
-
-    // Introduce new phi nodes in the second loop header to ensure
-    // exiting the first and jumping to the header of the second does not break
-    // the SSA property of the phis originally in the first loop. See also the
-    // comment above.
-    BasicBlock::iterator L1HeaderIP = FC1.Header->begin();
-    for (PHINode *LCPHI : OriginalFC0PHIs) {
-      int L1LatchBBIdx = LCPHI->getBasicBlockIndex(FC1.Latch);
-      assert(L1LatchBBIdx >= 0 &&
-             "Expected loop carried value to be rewired at this point!");
-
-      Value *LCV = LCPHI->getIncomingValue(L1LatchBBIdx);
-
-      PHINode *L1HeaderPHI =
-          PHINode::Create(LCV->getType(), 2, LCPHI->getName() + ".afterFC0");
-      L1HeaderPHI->insertBefore(L1HeaderIP);
-      L1HeaderPHI->addIncoming(LCV, FC0.Latch);
-      L1HeaderPHI->addIncoming(PoisonValue::get(LCV->getType()),
-                               FC0.ExitingBlock);
-
-      LCPHI->setIncomingValue(L1LatchBBIdx, L1HeaderPHI);
-    }
-
-    // Replace latch terminator destinations.
-    FC0.Latch->getTerminator()->replaceUsesOfWith(FC0.Header, FC1.Header);
-    FC1.Latch->getTerminator()->replaceUsesOfWith(FC1.Header, FC0.Header);
-
-    // Modify the latch branch of FC0 to be unconditional as both successors of
-    // the branch are the same.
-    simplifyLatchBranch(FC0);
-
-    // If FC0.Latch and FC0.ExitingBlock are the same then we have already
-    // performed the updates above.
-    if (FC0.Latch != FC0.ExitingBlock)
-      TreeUpdates.emplace_back(DominatorTree::UpdateType(
-          DominatorTree::Insert, FC0.Latch, FC1.Header));
-
-    TreeUpdates.emplace_back(DominatorTree::UpdateType(DominatorTree::Delete,
-                                                       FC0.Latch, FC0.Header));
-    TreeUpdates.emplace_back(DominatorTree::UpdateType(DominatorTree::Insert,
-                                                       FC1.Latch, FC0.Header));
-    TreeUpdates.emplace_back(DominatorTree::UpdateType(DominatorTree::Delete,
-                                                       FC1.Latch, FC1.Header));
+    rewireFusedHeaderPHIsAndLatches(FC0, FC1, OriginalFC0PHIs, TreeUpdates);
 
     // Update DT/PDT
     DTU.applyUpdates(TreeUpdates);
@@ -1537,52 +1609,7 @@ struct LoopFuser {
 
     DTU.flush();
 
-    // Is there a way to keep SE up-to-date so we don't need to forget the loops
-    // and rebuild the information in subsequent passes of fusion?
-    // Note: Need to forget the loops before merging the loop latches, as
-    // mergeLatch may remove the only block in FC1.
-    SE.forgetLoop(FC1.L);
-    SE.forgetLoop(FC0.L);
-
-    // Merge the loops.
-    SmallVector<BasicBlock *, 8> Blocks(FC1.L->blocks());
-    for (BasicBlock *BB : Blocks) {
-      FC0.L->addBlockEntry(BB);
-      FC1.L->removeBlockFromLoop(BB);
-      if (LI.getLoopFor(BB) != FC1.L)
-        continue;
-      LI.changeLoopFor(BB, FC0.L);
-    }
-    while (!FC1.L->isInnermost()) {
-      const auto &ChildLoopIt = FC1.L->begin();
-      Loop *ChildLoop = *ChildLoopIt;
-      FC1.L->removeChildLoop(ChildLoopIt);
-      FC0.L->addChildLoop(ChildLoop);
-    }
-
-    // Delete the now empty loop L1.
-    LI.erase(FC1.L);
-
-    // Forget block dispositions as well, so that there are no dangling
-    // pointers to erased/free'ed blocks. It should be done after mergeLatch()
-    // since merging the latches may affect the dispositions.
-    SE.forgetBlockAndLoopDispositions();
-
-    // Move instructions from FC0.Latch to FC1.Latch.
-    // Note: mergeLatch requires an updated DT.
-    mergeLatch(FC0, FC1);
-
-#ifndef NDEBUG
-    assert(!verifyFunction(*FC0.Header->getParent(), &errs()));
-    assert(DT.verify(DominatorTree::VerificationLevel::Fast));
-    assert(PDT.verify());
-    LI.verify(DT);
-    SE.verify();
-#endif
-
-    LLVM_DEBUG(dbgs() << "Fusion done:\n");
-
-    return FC0.L;
+    return finalizeFusedLoop(FC0, FC1);
   }
 
   /// Report details on loop fusion opportunities.
@@ -1758,60 +1785,7 @@ struct LoopFuser {
     TreeUpdates.emplace_back(DominatorTree::UpdateType(
         DominatorTree::Delete, FC1.Preheader, FC1.Header));
 
-    // Moves the phi nodes from the second to the first loops header block.
-    while (PHINode *PHI = dyn_cast<PHINode>(&FC1.Header->front())) {
-      if (SE.isSCEVable(PHI->getType()))
-        SE.forgetValue(PHI);
-      if (PHI->hasNUsesOrMore(1))
-        PHI->moveBefore(FC0.Header->getFirstInsertionPt());
-      else
-        PHI->eraseFromParent();
-    }
-
-    // Introduce new phi nodes in the second loop header to ensure
-    // exiting the first and jumping to the header of the second does not break
-    // the SSA property of the phis originally in the first loop. See also the
-    // comment above.
-    BasicBlock::iterator L1HeaderIP = FC1.Header->begin();
-    for (PHINode *LCPHI : OriginalFC0PHIs) {
-      int L1LatchBBIdx = LCPHI->getBasicBlockIndex(FC1.Latch);
-      assert(L1LatchBBIdx >= 0 &&
-             "Expected loop carried value to be rewired at this point!");
-
-      Value *LCV = LCPHI->getIncomingValue(L1LatchBBIdx);
-
-      PHINode *L1HeaderPHI =
-          PHINode::Create(LCV->getType(), 2, LCPHI->getName() + ".afterFC0");
-      L1HeaderPHI->insertBefore(L1HeaderIP);
-      L1HeaderPHI->addIncoming(LCV, FC0.Latch);
-      L1HeaderPHI->addIncoming(PoisonValue::get(LCV->getType()),
-                               FC0.ExitingBlock);
-
-      LCPHI->setIncomingValue(L1LatchBBIdx, L1HeaderPHI);
-    }
-
-    // Update the latches
-
-    // Replace latch terminator destinations.
-    FC0.Latch->getTerminator()->replaceUsesOfWith(FC0.Header, FC1.Header);
-    FC1.Latch->getTerminator()->replaceUsesOfWith(FC1.Header, FC0.Header);
-
-    // Modify the latch branch of FC0 to be unconditional as both successors of
-    // the branch are the same.
-    simplifyLatchBranch(FC0);
-
-    // If FC0.Latch and FC0.ExitingBlock are the same then we have already
-    // performed the updates above.
-    if (FC0.Latch != FC0.ExitingBlock)
-      TreeUpdates.emplace_back(DominatorTree::UpdateType(
-          DominatorTree::Insert, FC0.Latch, FC1.Header));
-
-    TreeUpdates.emplace_back(DominatorTree::UpdateType(DominatorTree::Delete,
-                                                       FC0.Latch, FC0.Header));
-    TreeUpdates.emplace_back(DominatorTree::UpdateType(DominatorTree::Insert,
-                                                       FC1.Latch, FC0.Header));
-    TreeUpdates.emplace_back(DominatorTree::UpdateType(DominatorTree::Delete,
-                                                       FC1.Latch, FC1.Header));
+    rewireFusedHeaderPHIsAndLatches(FC0, FC1, OriginalFC0PHIs, TreeUpdates);
 
     // All done
     // Apply the updates to the Dominator Tree and cleanup.
@@ -1834,52 +1808,7 @@ struct LoopFuser {
     DTU.deleteBB(FC0.ExitBlock);
     DTU.flush();
 
-    // Is there a way to keep SE up-to-date so we don't need to forget the loops
-    // and rebuild the information in subsequent passes of fusion?
-    // Note: Need to forget the loops before merging the loop latches, as
-    // mergeLatch may remove the only block in FC1.
-    SE.forgetLoop(FC1.L);
-    SE.forgetLoop(FC0.L);
-
-    // Merge the loops.
-    SmallVector<BasicBlock *, 8> Blocks(FC1.L->blocks());
-    for (BasicBlock *BB : Blocks) {
-      FC0.L->addBlockEntry(BB);
-      FC1.L->removeBlockFromLoop(BB);
-      if (LI.getLoopFor(BB) != FC1.L)
-        continue;
-      LI.changeLoopFor(BB, FC0.L);
-    }
-    while (!FC1.L->isInnermost()) {
-      const auto &ChildLoopIt = FC1.L->begin();
-      Loop *ChildLoop = *ChildLoopIt;
-      FC1.L->removeChildLoop(ChildLoopIt);
-      FC0.L->addChildLoop(ChildLoop);
-    }
-
-    // Delete the now empty loop L1.
-    LI.erase(FC1.L);
-
-    // Forget block dispositions as well, so that there are no dangling
-    // pointers to erased/free'ed blocks. It should be done after mergeLatch()
-    // since merging the latches may affect the dispositions.
-    SE.forgetBlockAndLoopDispositions();
-
-    // Move instructions from FC0.Latch to FC1.Latch.
-    // Note: mergeLatch requires an updated DT.
-    mergeLatch(FC0, FC1);
-
-#ifndef NDEBUG
-    assert(!verifyFunction(*FC0.Header->getParent(), &errs()));
-    assert(DT.verify(DominatorTree::VerificationLevel::Fast));
-    assert(PDT.verify());
-    LI.verify(DT);
-    SE.verify();
-#endif
-
-    LLVM_DEBUG(dbgs() << "Fusion done:\n");
-
-    return FC0.L;
+    return finalizeFusedLoop(FC0, FC1);
   }
 };
 } // namespace
diff --git a/llvm/test/Transforms/LoopFusion/loop_invariant.ll b/llvm/test/Transforms/LoopFusion/loop_invariant.ll
index ea682ac363aad..3091e622cfe4a 100644
--- a/llvm/test/Transforms/LoopFusion/loop_invariant.ll
+++ b/llvm/test/Transforms/LoopFusion/loop_invariant.ll
@@ -60,3 +60,84 @@ body2:  ; preds = %pre2, %body2
 exit:
   ret void
 }
+
+; Test idempotent store-store pairs: both loops write the same value to
+; Safe to fuse despite MayAlias.
+
+define void @idempotent_same_arg(ptr %a, ptr %b, i32 %n, i32 %val) {
+; CHECK-DA: Performing Loop Fusion on function idempotent_same_arg
+; CHECK-DA: Fusion is performed
+entry:
+  %cmp = icmp sgt i32 %n, 0
+  br i1 %cmp, label %for.body.preheader, label %for.cond2.preheader
+
+for.body.preheader:
+  %wide.trip.count = zext nneg i32 %n to i64
+  br label %for.body
+
+for.body:
+  %iv = phi i64 [ 0, %for.body.preheader ], [ %iv.next, %for.body ]
+  %gep.a = getelementptr inbounds i32, ptr %a, i64 %iv
+  store i32 %val, ptr %gep.a, align 4
+  %iv.next = add nuw nsw i64 %iv, 1
+  %exit = icmp eq i64 %iv.next, %wide.trip.count
+  br i1 %exit, label %for.cond2.preheader, label %for.body
+
+for.cond2.preheader:
+  %cmp2 = icmp sgt i32 %n, 0
+  br i1 %cmp2, label %for.body5.preheader, label %for.cond.cleanup4
+
+for.body5.preheader:
+  %wide.trip.count2 = zext nneg i32 %n to i64
+  br label %for.body5
+
+for.body5:
+  %iv2 = phi i64 [ 0, %for.body5.preheader ], [ %iv2.next, %for.body5 ]
+  %gep.b = getelementptr inbounds i32, ptr %b, i64 %iv2
+  store i32 %val, ptr %gep.b, align 4
+  %iv2.next = add nuw nsw i64 %iv2, 1
+  %exit2 = icmp eq i64 %iv2.next, %wide.trip.count2
+  br i1 %exit2, label %for.cond.cleanup4, label %for.body5
+
+for.cond.cleanup4:
+  ret void
+}
+
+define void @different_values_no_fusion(ptr %a, ptr %b, i32 %n, i32 %t) {
+; CHECK-DA: Performing Loop Fusion on function different_values_no_fusion
+; CHECK-DA: Memory dependencies do not allow fusion!
+entry:
+  %cmp = icmp sgt i32 %n, 0
+  br i1 %cmp, label %for.body.preheader, label %for.cond2.preheader
+
+for.body.preheader:
+  %wide.trip.count = zext nneg i32 %n to i64
+  br label %for.body
+
+for.body:
+  %iv = phi i64 [ 0, %for.body.preheader ], [ %iv.next, %for.body ]
+  %gep.a = getelementptr inbounds i32, ptr %a, i64 %iv
+  store i32 %t, ptr %gep.a, align 4
+  %iv.next = add nuw nsw i64 %iv, 1
+  %exit = icmp eq i64 %iv.next, %wide.trip.count
+  br i1 %exit, label %for.cond2.preheader, label %for.body
+
+for.cond2.preheader:
+  %cmp2 = icmp sgt i32 %n, 0
+  br i1 %cmp2, label %for.body5.preheader, label %for.cond.cleanup4
+
+for.body5.preheader:
+  %wide.trip.count2 = zext nneg i32 %n to i64
+  br label %for.body5
+
+for.body5:
+  %iv2 = phi i64 [ 0, %for.body5.preheader ], [ %iv2.next, %for.body5 ]
+  %gep.b = getelementptr inbounds i32, ptr %b, i64 %iv2
+  store i32 42, ptr %gep.b, align 4
+  %iv2.next = add nuw nsw i64 %iv2, 1
+  %exit2 = icmp eq i64 %iv2.next, %wide.trip.count2
+  br i1 %exit2, label %for.cond.cleanup4, label %for.body5
+
+for.cond.cleanup4:
+  ret void
+}



More information about the llvm-commits mailing list