[PATCH] D28534: [LCSSA] Don't let SSAUpdater to break LCSSA during LCSSA construction.

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added inline comments.


================
Comment at: lib/Transforms/Utils/LCSSA.cpp:81
   // expensive, and we're not mutating the loop structure.
   SmallDenseMap<Loop*, SmallVector<BasicBlock *,1>> LoopExitBlocks;
 
----------------
Note: this cache become less effective since we're not reusing it for the entire worklist anymore.


================
Comment at: lib/Transforms/Utils/LCSSA.cpp:226
       // Reprocess each PHI instruction.
       Worklist.push_back(PostProcessPN);
     }
----------------
Why is it legal to re-use the same `SSAUpdater` to rewrite uses for these newly inserted PHIs?  Asking mostly because earlier `SSAUpdate` would get reset for each iteration of the inner loop on the worklist.


================
Comment at: lib/Transforms/Utils/LCSSA.cpp:251
+  bool Changed = false;
+  while (!Worklist.empty()) {
+    Instruction *I = Worklist.pop_back_val();
----------------
(Fix separately without review if you agree) Why not just a range for over an `ArrayRef<Instruction *>`?


https://reviews.llvm.org/D28534