[PATCH] D75013: [LoopTerminology] Rotated Loops

[Kit Barton via Phabricator via llvm-commits]

kbarton added a comment.

I went through the recordings you cited, and as you expected these are mistakes.
First, the comment I made about loop latch having a single //predecessor// was a typo introduced when converting slides. That should have been //successor//, to match with the slides I presented at EuroLLVM.
The comment about single successor I got from the original patch for loop rotation (https://reviews.llvm.org/D22630). However, as @Meinersbur pointed out earlier, this comment doesn't make sense because the latch has to have two successors: the loop header and the loop exit block.

I apologize for the confusion. Hopefully things are more clear now.



================
Comment at: llvm/docs/LoopTerminology.rst:206
+above, that would be:
+
+.. code-block:: C
----------------
As a way to articulate this warning concretely, in my experience when you use `unsigned` as the parameter type, LLVM is able to prove the guard is not necessary and thus will generate the example you give above (//i.e.//, it knows that n is always greater than zero and thus can fold the guard away. 

On the other hand, if you use `int`, it generally //cannot// prove the guard is not necessary (in my experience) and thus will always create the guard.




================
Comment at: llvm/docs/LoopTerminology.rst:222
+in LLVM IR while also providing a graphical reprensentation
+of the control-flow graphs (CFG). You can get the same graphical
+results by utilizing the `view-cfg <passes-view-cfg>` pass.
----------------
nit: control-flow graphs -> Control Flow Graphs (CFG)
I tend to capitalize the words that are used in acronyms. I don't know if this is grammatically correct or just common practice. 


================
Comment at: llvm/docs/LoopTerminology.rst:289
+  resulting in the following:
+
+.. code-block:: none
----------------
A couple comments on this:

  # My understanding was that loop rotate will always generate the guard, and if it can prove the guard is not needed then it will remove it.
  # Even if it is not the case, I personally don't like giving an example and then saying in text below it "this isn't actually legal, we need to do this also". Would it be possible to rewrite this to give the complete example below, and then follow-up with something along the lines of: "if the compiler can prove that the guard condition always evaluates to true, it will remove the guard altogether."?



================
Comment at: llvm/docs/LoopTerminology.rst:332
+in the original loop. That happens in the case that the loop is not
+executed at all.
----------------
What are you referring to by "this form"? Is this rotated loops in general, or rotated loops that have a guard?

I think this could be made more concrete by referring back to the original example. In the original loop, `entry:` is the preheader for the loop, but trying to hoist an instruction from the loop body is incorrect if the loop body never executes. With a guarded loop, on the other hand, moving an instruction from `body:` to `loop.preheader:` is legal because `loop.preheader:` is only executed if `body:` is executed at least once (figure GuardedLoop). When the compiler can prove that the guard is not needed, it is still legal to hoist instructions from `body:` to `entry:` because it is known that `body:` will execute at least once (figure RotatedLoop). 


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

https://reviews.llvm.org/D75013