[PATCH] D75013: [LoopTerminology] Rotated Loops

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: llvm/docs/LoopTerminology.rst:195-196
+
+This transformation canonicalizes the loop latch to have
+a single successor, which implies that the loop latch
+is also an exiting block. It is done by the `loop-rotate`
----------------
baziotis wrote:
> Meinersbur wrote:
> > [serious] By definition, a latch has a backedge to the header. If the latch had just a single successor, there could not be another edge to outside the loop.
> > 
> > Using C code to describe the effect is higher level than the IR-level it is actually performed on. What a latch is is not obvious in the C code. LoopRotate also copies the header block, which might be interesting to mention.
> > [serious] By definition, a latch has a backedge to the header. If the latch had just a single successor, there could not be another edge to outside the loop.
> 
> Yes, given that the latch contains the condition (as in a do-while loop), that was not so smart on my part. :P
> But honestly, the 2 videos confused me in this part:
> - On the one video it says that the latch has a single successor. I assume was a typo.
> - On the other, it says that it has a single predecessor. Which, in the example given, I don't think is true. Besides that,
> I don't see why having a single predecessor is important. What do I miss?
> 
> > Using C code to describe the effect is higher level than the IR-level it is actually performed on. What a latch is is not obvious in the C code. LoopRotate also copies the header block, which might be interesting to mention.
> 
> Yes, I'll change it to IR. How about if I put view-cfg-produced image?
Could you point me to where in the videos this statement is made? It is totally possible that we make mistakes as well.

I also would not see why the latch would need to have a single predecessor.

> How about if I put view-cfg-produced image?

In image illustration would be nice.


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

https://reviews.llvm.org/D75013