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

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
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;
+
----------------
reames wrote:
> 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).
Or just inline mergeLoop and restore the check removed by cce6712519 for the parent loop.  That's the change which appears to have introduced this compile time regression.  :)

Yeah for compile time problems which go unfound for 20 months...


https://reviews.llvm.org/D51715