[PATCH] D51718: Update MemorySSA in LoopRotate.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea marked an inline comment as done.
asbirlea 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);
----------------
fhahn wrote:
> 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.
It's correct, but unnecessary. Thank you for noticing!
That verification can be removed, since there are additional verificatons inside `LoopRotate::rotateLoop` that should cover the need to verify after the `LR.processLoop(L);` call.



Repository:
  rL LLVM

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

https://reviews.llvm.org/D51718