[PATCH] D123288: [LoopSink] Require MemorySSA

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added a comment.

Added nits, but happy if this goes in as is too.



================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:161
+                                     SinkAndHoistLICMFlags &Flags);
+static bool pointerInvalidatedByBlock(BasicBlock &BB, MemorySSA &MSSA,
+                                      MemoryUse &MU);
----------------
These two APIs could be made consistent to either have the Loop and BB as the first argument, or MSSA and MU. Not worth bike-shedding though..


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:1220
         for (Value *Op : CI->args())
           if (Op->getType()->isPointerTy()) {
+            if (pointerInvalidatedByLoop(
----------------
The two if conditions can be `&&` appended; I believe this was the case before I split the MSSA and AST code paths.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123288/new/

https://reviews.llvm.org/D123288