[PATCH] D125990: [LSR] Fix bug for optimizing unused IVs to final values

[Zaara Syeda via Phabricator via llvm-commits]

syzaara created this revision.
syzaara added reviewers: LoopOptWG, uabelho.
Herald added a subscriber: hiraditya.
Herald added a project: All.
syzaara requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This is a fix for a crash reported for https://reviews.llvm.org/D118808
The fix is to only consider PHINodes which are induction phis.


https://reviews.llvm.org/D125990

Files:
  llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
  llvm/test/Transforms/LoopStrengthReduce/remove_scev_indvars.ll


Index: llvm/test/Transforms/LoopStrengthReduce/remove_scev_indvars.ll
===================================================================
--- llvm/test/Transforms/LoopStrengthReduce/remove_scev_indvars.ll
+++ llvm/test/Transforms/LoopStrengthReduce/remove_scev_indvars.ll
@@ -55,3 +55,39 @@
   %indvars.iv.unr = phi i64 [%iv.prol.lcssa, %for.exit]
   ret void
 }
+
+define void @testNonIndVarPhi() {
+cont5820:
+  br label %for.cond5821
+
+for.cond5821:                                     ; preds = %cont5825, %cont5820
+  %0 = phi i32 [ 0, %cont5825 ], [ 1, %cont5820 ]
+  br label %cont5825
+
+cont5825:                                         ; preds = %for.cond5821
+  br i1 false, label %for.cond5821, label %for.cond6403
+
+for.cond6403:                                     ; preds = %dead, %cont5825
+  %1 = phi i32 [ %.lcssa221, %dead ], [ 0, %cont5825 ]
+  br label %for.cond6418
+
+for.cond6418:                                     ; preds = %cont6497, %for.cond6403
+  %2 = phi i32 [ %0, %cont6497 ], [ %1, %for.cond6403 ]
+  %3 = phi i64 [ 1, %cont6497 ], [ 0, %for.cond6403 ]
+  %cmp6419 = icmp ule i64 %3, 0
+  br i1 %cmp6419, label %cont6497, label %for.end6730
+
+cont6497:                                         ; preds = %for.cond6418
+  %conv6498 = sext i32 %2 to i64
+  br label %for.cond6418
+
+for.end6730:                                      ; preds = %for.cond6418
+; Check that we don't make changes for phis which are not considered
+; induction variables
+; CHECK: %.lcssa221 = phi i32 [ %2, %for.cond6418 ]
+  %.lcssa221 = phi i32 [ %2, %for.cond6418 ]
+  ret void
+
+dead:                                             ; No predecessors!
+  br label %for.cond6403
+}
Index: llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -66,6 +66,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Analysis/AssumptionCache.h"
+#include "llvm/Analysis/IVDescriptors.h"
 #include "llvm/Analysis/IVUsers.h"
 #include "llvm/Analysis/LoopAnalysisManager.h"
 #include "llvm/Analysis/LoopInfo.h"
@@ -5598,9 +5599,10 @@
     DeadInsts.emplace_back(OperandIsInstr);
 }
 
-// Check if there are any loop exit values which are only used once within the
-// loop which may potentially be optimized with a call to rewriteLoopExitValue.
-static bool LoopExitValHasSingleUse(Loop *L) {
+// Check if there are any loop exit values which are induction variables in the
+// loop and are only used once within the loop (for IV updates). These can then
+// potentially be optimized with a call to rewriteLoopExitValue.
+static bool IsIndVarWithSingleUse(Loop *L, ScalarEvolution &SE) {
   BasicBlock *ExitBB = L->getExitBlock();
   if (!ExitBB)
     return false;
@@ -5611,6 +5613,14 @@
 
     BasicBlock *Pred = ExitPhi.getIncomingBlock(0);
     Value *IVNext = ExitPhi.getIncomingValueForBlock(Pred);
+    if (isa<PHINode>(IVNext)) {
+      PHINode *PN = cast<PHINode>(IVNext);
+      InductionDescriptor ID;
+      // We only want to consider induction PHIs.
+      if (!InductionDescriptor::isInductionPHI(PN, L, &SE, ID))
+        return false;
+    }
+
     // One use would be the exit phi node, and there should be only one other
     // use for this to be considered.
     if (IVNext->getNumUses() == 2)
@@ -6624,7 +6634,7 @@
   // When this is the case, if the exit value of the IV can be calculated using
   // SCEV, we can replace the exit block PHI with the final value of the IV and
   // skip the updates in each loop iteration.
-  if (L->isRecursivelyLCSSAForm(DT, LI) && LoopExitValHasSingleUse(L)) {
+  if (L->isRecursivelyLCSSAForm(DT, LI) && IsIndVarWithSingleUse(L, SE)) {
     SmallVector<WeakTrackingVH, 16> DeadInsts;
     const DataLayout &DL = L->getHeader()->getModule()->getDataLayout();
     SCEVExpander Rewriter(SE, DL, "lsr", false);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D125990.430727.patch
Type: text/x-patch
Size: 3985 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220519/8ceb8405/attachment.bin>