[PATCH] D82108: [IndVarSimplify] Don't replace IV user with unsafe loop-invariant (PR45360)

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added a comment.

Generally looks ok. Please use `isSafeToExpandAt` because the context can give us more information about divisor being non-zero.



================
Comment at: llvm/lib/Transforms/Utils/SimplifyIndVar.cpp:660
 
 /// Replace the UseInst with a constant if possible.
 bool SimplifyIndvar::replaceIVUserWithLoopInvariant(Instruction *I) {
----------------
This comment doesn't reflect what this method does, can you pls also fix it along, like, "Replace the UseInst with a loop invariant expression if it is safe"?


================
Comment at: llvm/lib/Transforms/Utils/SimplifyIndVar.cpp:675
 
+  if (!isSafeToExpand(S, *SE)) {
+    LLVM_DEBUG(dbgs() << "INDVARS: Can not replace IV user: " << *I
----------------
Please `isSafeToExpandAt`.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyIndVar.cpp:677
+    LLVM_DEBUG(dbgs() << "INDVARS: Can not replace IV user: " << *I
+                      << " with loop invariant: " << *S << '\n');
+    return false;
----------------
`with non-speculable loop invariant` maybe?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82108/new/

https://reviews.llvm.org/D82108