[PATCH] D75013: [LoopTerminology] Rotated Loops

Stefanos Baziotis via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 4 03:59:03 PST 2020


baziotis marked 3 inline comments as done.
baziotis added a comment.

In D75013#1904564 <https://reviews.llvm.org/D75013#1904564>, @kbarton wrote:

> 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.


Yes, things are quite clearer now, thanks for the clarifications!



================
Comment at: llvm/docs/LoopTerminology.rst:206
+above, that would be:
+
+.. code-block:: C
----------------
kbarton wrote:
> 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.
> 
> 
Yes, I'll follow with this clarification.


================
Comment at: llvm/docs/LoopTerminology.rst:289
+  resulting in the following:
+
+.. code-block:: none
----------------
kbarton wrote:
> 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."?
> 
> 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.
Thanks, I didn't know that.

Regarding the rest, generally I agree but in this case it may be a big jump from the `for` version to the `loop-rotate` result because it will also `loo-simplify` it, what do you think ?
We could do this: After the `for` LLVM IR, follow with "this is how we could convert that in do-while style loop", basically implying that this is not what `loop-rotate` will do. Follow with the notes and //then// "here's what loop-rotate will do. It inserted a guard because ... and also simplified it ..."


================
Comment at: llvm/docs/LoopTerminology.rst:332
+in the original loop. That happens in the case that the loop is not
+executed at all.
----------------
kbarton wrote:
> 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). 
"this form" being the result of `loop-rotate`. This can be without a guard under circumstances right?
Given that `loop-rotate` has deduced whether we need a guard or not, moving things in the preheader should be valid whether it has a guard or not (i.e. if it doesn't have a guard, it is because `loop-rotate` has deduced that the loop will execute at least once).

Which is basically what you referred to so I'll try to make clear that moving things in the preheader after `loop-rotate` is always correct whether it has a guard or not.


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

https://reviews.llvm.org/D75013





More information about the llvm-commits mailing list