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

[Alex Gatea via Phabricator via llvm-commits]

alexgatea added a comment.

In D129599#3749005 <https://reviews.llvm.org/D129599#3749005>, @drcut wrote:

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

I appreciate your prompt response. A couple of points ...
The profile data is updated every time I run the spec benchmark, so it's accurate. And the 30% degradation is significant.
Thank you for pointing out that example; I agree that isColdBlock should be global. 
I actually called isFunctionColdInCallGraph on the function in question and it returns false, so this function is hot. I also checked the ColdCount of the loop in question and it is 1 (in particular non-zero); another hot block in this same function has a ColdCount of 2000. My guess is that the block in question is cold relative to other blocks in the function (by a factor of ~2000 I guess) but is still itself significant. And of course, this cold block analysis doesn't take into consideration how much the loop itself is optimizable if unswitched so it could still be beneficial to unswitch even though it is a cold block.
Please let me know your thoughts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129599