[PATCH] D129599: [SimpleLoopUnswitch] Skip non-trivial unswitching of cold loops

[Ruobing Han via Phabricator via llvm-commits]

drcut added a comment.

@alexgatea Thanks for the feedback. 
isColdBlock should be global. One example is in (https://llvm.org/doxygen/ProfileSummaryInfo_8cpp_source.html#l00142), which uses isColdBlock to check whether a function is cold or not. So I assume the issue is the loop is non-cold in real case, but was concerned as cold in the profile data. I kindly suggest updating the profile data to see if there is still any degradation.
Please correct me if I am wrong. Thanks

In D129599#3748815 <https://reviews.llvm.org/D129599#3748815>, @alexgatea wrote:

> I noticed significant performance degradation (~30%) on a spec benchmark due to this commit. isColdBlock doesn't seem to work as expected, because it considered cold a loop that was in a hot function through the profile.
>
> For example, it could be the case that the loop in question is nested and is vectorizable after being unswitched, so not unswitching it can result in performance degradation if the surrounding function is hot.
>
>   void hotFunction(int M, int N, int * A, int *B, int *C){
>   for (j = 0; j < M; j++)
>      for (i=0; i < N; i++) {
>         A[i] = B[i] + C[i]
>         if (cond) do_something();
>      }
>   }
>
> Could you please revert this commit while you work on replacing the isColdBlock check with something stronger?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129599