[llvm] [polly] [SimpleLoopUnswitch] Don't use BlockFrequencyInfo to skip cold loops (PR #159522)

[Luke Lau via llvm-commits]

lukel97 wrote:

> Agree, this change isn't going to be equivalent. I'm a little unclear of the problem being solved by this PR. What is the main motivation? There's a list of benefits but I'm not sure how important they are.

The main motivation is to remove the last use of BlockFrequencyInfo from within a loop pass, because I'm not sure if there's much value in relying on a potentially inaccurate result given that it's only preserved lossily.

I noticed this originally when some of the lossy results escaped into the loop vectorizer in #157888, causing BFI to return some bogus frequencies. Not off by one or two etc, but returning the frequencies for different blocks altogether since the CFG had been modified. I'm a bit worried that SimpleLoopUnswitch is also affected by this e.g. thinking a loop is cold when it's not. 

There is a secondary motivation that avoiding code size increases should be handled by PGOForceFunctionAttrsPass. I think it's somewhat coincidental that the patch checked that the loops were cold instead of the function. As Nikita mentions, fb45f3c9486f5d9e3003db95386432562b23577c seems to suggest that the original author was fine with checking the function.


https://github.com/llvm/llvm-project/pull/159522