[PATCH] D124926: [LoopInterchange] New cost model for loop interchange

[Congzhe Cao via Phabricator via llvm-commits]

congzhe added a comment.

In D124926#3608830 <https://reviews.llvm.org/D124926#3608830>, @congzhe wrote:

> In D124926#3608260 <https://reviews.llvm.org/D124926#3608260>, @bmahjour wrote:
>
>> The cost overflow is a known issue (see https://github.com/llvm/llvm-project/issues/55233). I wonder if specifying a larger cache line size (eg. `-cache-line-size=128` or 256 ) would workaround the ubsan failures for now. Otherwise we'd have to fix 55233 before recommitting this, I guess.
>
> The test case that fails upstream test, i.e., `test/Transforms/LoopInterchange/interchangeable-innerloop-multiple-indvars.ll`, is not supposed to result in large values in loop cache analysis calculation. The loop costs associated with those loops should be pretty small, and overflow is not supposed to occur. This is a bit strange, and on my local machine all tests did pass. We may need to dig deeper to find why it fails upstream build bot, nevertheless not being able to reproduce the failure has caused a bit of headache for me.

Okay I missed the fact that this is a sanitizer test, it makes more sense to fail the test now since potentially loop cache analylsis could overflow. But I'm still not entirely clear - if loop cache analysis suffers from potential overflow it should have failed the tests in `test/LoopCacheAnalysis/` quite early on at the first place. Why did `test/LoopCacheAnalysis/` not fail the sanitizer tests before but `test/LoopInterchange/` tests fail now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124926