[PATCH] D24934: [LICM] Add support of a new optimization case to Loop Versioning for LICM + code clean up

[Ashutosh Nema via llvm-commits]

ashutosh.nema added a comment.

Thanks Evgeny for working on this.

Currently when pointers bounds are unknown we don’t consider them
for alias check and we avoid versioning, but this change introduces partial
alias check, it may be risky for applications, wondering what’s the purpose partial
alias check.


================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:403
@@ +402,3 @@
+/// \return true if a loaded pointer can have bounds.
+bool LoopVersioningLICM::canLoadedPtrHaveBounds(const LoadInst * Load) {
+  assert(Load);
----------------
This function looks for very specific pattern, need to generalize.

================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:431
@@ +430,3 @@
+
+  if (!Load->hasOneUse() && !areOtherUsersOnlyMemOps(Load, GEP)) {
+    DEBUG(dbgs() << "     Failed: Non-memory operation different from GEP uses Load\n");
----------------
Why restricting to single load & its users as memory operations & GEP ?

================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:446
@@ +445,3 @@
+
+  if (!GEP->hasOneUse()) {
+    DEBUG(dbgs() << "     Failed: GEP has more than one users.\n");
----------------
Why one use ?

================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:489
@@ +488,3 @@
+  // should be used for making decision.
+  if (CurLoop->getParentLoop()) {
+    DEBUG(dbgs() << "    Loop nests are not supported.\n");
----------------
Current LoopVersioningLICM supports loop nest, i.e. it targets innermost loop.
This behavior should be keep supported.


================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:496
@@ +495,3 @@
+    const LoadInst * Load = dyn_cast<LoadInst>(Ptr);
+    if (!Load) {
+      DEBUG(dbgs() << "    It is not a loaded a pointer: " << *Ptr << "\n");
----------------
Earlier collected write to memory as well, why don't want to consider them here ?


https://reviews.llvm.org/D24934