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

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added a comment.

> Thanks for the comment, I see your concern. Would you like me to continue D127342 <https://reviews.llvm.org/D127342>, or would you like me to still keep the cache line size option under `LoopCacheCost.cpp` as we did in this patch? To avoid breaking computers with no cache, I could update `CLS` in `LoopCacheCost.cpp` to the following (I may need to change to wording a little bit) :
>
>   CLS = DefaultCacheLineSize.getNumOccurrences() > 0  ? DefaultCacheLineSize : TTI.getCacheLineSize()
>
> and pass -cache-line-size=64 to all loop interchange tests. This way the buildbot test failures can be avoided, and in the meantime for a realworld computer with no cache it won't break.

I slightly prefer to continue with D127342 <https://reviews.llvm.org/D127342>, assuming you bring it up with the wider community (eg. through Discourse) and there are no objections. Not having cache is an exception (at best) rather than the norm, so it doesn't make much sense to me that the default assumes no cache. 
If that doesn't pan out I'm ok with your second suggestion, but we'd also need to teach LoopCacheCost to handle CLS == 0 (otherwise it would divide by zero).


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

https://reviews.llvm.org/D124926