[PATCH] D99354: [SimpleLoopUnswitch] Port partially invariant unswitch from LoopUnswitch to SimpleLoopUnswitch

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D99354#2763024 <https://reviews.llvm.org/D99354#2763024>, @jaykang10 wrote:

> The below bugs are fixed.
> https://bugs.llvm.org/show_bug.cgi?id=50279
> https://bugs.llvm.org/show_bug.cgi?id=50302
>
> Even if this patch does not add the processed loop to loop pass manager again, the later loop transformations create loop with the header, which has partially invariant instructions, and add it to loop pass manager again. The loop pass manager runs SimpleLoopUnswitchPass with the loop again and it sometimes causes endless unswitch with some CFGs.
>
> In order to avoid the endless unswitch, the new change removes the partially invariant instruction from header as below.
>
>   //    +--------------------+
>   //    |     preheader      |
>   //    |  %a = load %ptr    |
>   //    +--------------------+
>   //             |     /----------------\
>   //    +--------v----v------+          |
>   //    |      header        |---\      |
>   //    | %c = phi %a, %b    |   |      |
>   //    | %cond = cmp %c, .. |   |      |
>   //    | br %cond           |   |      |
>   //    +--------------------+   |      |
>   //             |               |      |
>   //    +--------v-----------+   |      |
>   //    |  store %ptr        |   |      |
>   //    +--------------------+   |      |
>   //             |               |      |
>   //    +--------v-----------<---/      |
>   //    |       latch        >----------/
>   //    |  %b = load %ptr    |
>   //    +--------------------+

Hm, this seems quite fragile, as another pass may decide to move the load to the header again. In the legacy pass manager, the loop is annotated with metadata to avoid partially unswithcing the same loop multiple times (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Scalar/LoopUnswitch.cpp#L849) Can we do the same here?



================
Comment at: llvm/test/Transforms/SimpleLoopUnswitch/endless-unswitch.ll:1
+; RUN: opt -O3 -S < %s | FileCheck %s
+
----------------
We should avoid adding tests using `-O3` here. Can we reproduce the issue by running `simple-loop-unswtich` twice? Also it would be good to clean up the test a bit.


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

https://reviews.llvm.org/D99354