[PATCH] D75013: [LoopTerminology] Rotated Loops

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis marked an inline comment as done.
baziotis added inline comments.


================
Comment at: llvm/docs/LoopTerminology.rst:325
+because loop-rotate triggers the
+:ref:`-loop-simplify <loop-terminology-loop-simplify>` pass to run.
+In this case, it inserted the %loop.preheader basic block so
----------------
Meinersbur wrote:
> baziotis wrote:
> > Meinersbur wrote:
> > > [nit] please make clear that -loop-simplify runs //before// -loop-rotate.
> > Actually, related to the other comment I wrote, it seems to me that running LoopSimplify before LoopRotate will have no effect. Probably this effect was done because LoopSimplify was ran after. And since LoopRotate broke the form, the //after// LoopSimplify is the one that changed it.
> LoopRotation, at least in the legacy pass manager, depends on LoopSimplify (https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Scalar/LoopRotation.cpp#L86), so it will have run when entering LoopRotation. There might be other executions of LoopSimplify added to the pass manager (which then will have no effect)
> 
> Also LoopRotation bails out if not in simplified form: https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp#L324
> 
> Moreover, LoopRotation ensures simplified form after rotation: https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp#L561
Yes, I agree with all of that! Actually, I probably stated it incorrectly:
I did not mean that a LoopSimplify won't run before since it is added automatically by the pass manager for LoopPasses.
And because LoopRotate works only on simplified loops.
But in this example, it will have no effect. So, there has to be an //after// LoopSimplify for this effect. And specifically, I didn't see this, thanks:
> Moreover, LoopRotation ensures simplified form after rotation: https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp#L561

So, I'll update it to make that clear.


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

https://reviews.llvm.org/D75013