[PATCH] D51718: Update MemorySSA in LoopRotate.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/trunk/lib/Transforms/Utils/LoopRotationUtils.cpp:682-687
+  LoopRotate LR(Threshold, LI, TTI, AC, DT, SE, MSSAU, SQ, RotationOnly,
+                IsUtilMode);
+  if (MSSAU && VerifyMemorySSA)
+    MSSAU->getMemorySSA()->verifyMemorySSA();
 
   return LR.processLoop(L);
----------------
lebedev.ri wrote:
> @asbirlea @fhahn 
> 
> Is this actually correct?
> Don't you want to run verification after actually doing the changes?
> I.e. shouldn't this be
> ```
> {
>   if (MSSAU && VerifyMemorySSA)
>     MSSAU->getMemorySSA()->verifyMemorySSA();
>   LoopRotate LR(Threshold, LI, TTI, AC, DT, SE, MSSAU, SQ, RotationOnly,
>                 IsUtilMode);
>   bool Changed = LR.processLoop(L);
>   if (MSSAU && VerifyMemorySSA)
>     MSSAU->getMemorySSA()->verifyMemorySSA();
>   return Changed;
> }
> ```
Yeah the code in the patch doesn't look right, given that the actual modifications happen in processLoop.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D51718