[PATCH] D47407: [LoopInstSimplify] Re-implement the core logic of loop-instsimplify to be both simpler and substantially more efficient.

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy accepted this revision.
sanjoy added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopInstSimplify.cpp:92
 
-        if (IsSubloopHeader && !isa<PHINode>(I))
-          break;
-      }
-
-      // Add all successors to the worklist, except for loop exit blocks and the
-      // bodies of subloops. We visit the headers of loops so that we can
-      // process
-      // their phis, but we contract the rest of the subloop body and only
-      // follow
-      // edges leading back to the original loop.
-      for (succ_iterator SI = succ_begin(BB), SE = succ_end(BB); SI != SE;
-           ++SI) {
-        BasicBlock *SuccBB = *SI;
-        if (!Visited.insert(SuccBB).second)
+        if (!ToSimplify->empty() && !ToSimplify->count(&I))
           continue;
----------------
How about pulling out `ToSimplify->empty()` into a `bool IsFirstIteration`?  I think that will make the intent clearer.


================
Comment at: llvm/lib/Transforms/Scalar/LoopInstSimplify.cpp:118
+          // we won't have visited it yet.
+          if (!ToSimplify->empty() && L.contains(UserI))
+            ToSimplify->insert(UserI);
----------------
Might be worth asserting that if `!L.contains(UserI)` then `UserI` is a PHI node.  Up to you.


Repository:
  rL LLVM

https://reviews.llvm.org/D47407