[PATCH] D51715: [LICM] Avoid duplicate work during building AliasSetTracker

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

This looks correct to me, but this is so glaringly bad I'd like a second set of eyes to make sure I'm not missing something.  Can someone else confirm?



================
Comment at: lib/Transforms/Scalar/LICM.cpp:1545
   std::unique_ptr<AliasSetTracker> CurAST;
-  SmallVector<Loop *, 4> RecomputeLoops;
-  auto mergeLoop = [&CurAST](Loop *L) {
-    // Loop over the body of this loop, looking for calls, invokes, and stores.
-    for (BasicBlock *BB : L->blocks())
-      CurAST->add(*BB); // Incorporate the specified basic block
-  };
+  DenseSet<BasicBlock *> HandledBBs;
+
----------------
I'm fine with this implementation, but if you wanted, you could structure this one of two ways:
1) Use a SmallVector<BasicBlock> since the sets of BBs are distinct by assumption.
2) Keep track of which loops are handle, then check the containing loop for each BB using LoopInfo (it has a map from BB to innermost containing loop).


https://reviews.llvm.org/D51715