[PATCH] D22630: Loop rotation

[Aditya Kumar via llvm-commits]

hiraditya added a comment.

In https://reviews.llvm.org/D22630#491737, @mzolotukhin wrote:

> Hi Aditya,
>
> Thanks for working on this! I've been working on some improvements in loop-rotation too, though it looks like we're targeting slightly different cases.
>
> I have a question inspired by the test `pr7447.ll`: I don't understand why we should rotate these loops (looking at just CFG). My guess is that helping GVN/LICM/etc would be just a luck in this case, and e.g. if you run loop-rotate twice on this test, you'd get the original IR, or something similar. What I'm getting at is that if we want to prepare IR for some other passes, we need to make sure we only touch cases where these passes would benefit. We can't just blindly rotate more loops. Or am I missing something? What is the current motivation to rotate or not rotate the loop?
>
> FWIW, this is the CFG of the test from `pr7447.ll`:
>
>         |
>         v
>   (  for.cond  )
>    |    |    ^
>    |    |    |
>    |    v    |
>    | ( for.cond1 )
>    |    |    ^
>    |    |    |
>    |    v    |
>    | ( land.rhs )
>    |     |
>    |     |
>    v     v
>   ( return )
>
>
> Thanks,
> Michael


I agree, we should not blindly rotate loops just because we can. This patch can rotate more loop structures so your argument is more valid. Maybe, this pass could be provided on-demand basis so that only those loops should be rotated which might benefit. As it is, the logic to decide whether to rotate or not could be added in isProfitableToRotate. I guess that would also depend on case by case basis as we come across more of them.


https://reviews.llvm.org/D22630