[PATCH] D128877: [LoopCacheAnalysis] Fix a type mismatch bug in cost calculation

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: llvm/lib/Analysis/LoopCacheAnalysis.cpp:479-480
+  Type *WiderType = SE.getWiderType(Coeff->getType(), ElemSize->getType());
+  // TODO: Extend more explicitly based on whether Coeff and ElemSize are
+  // positive or negative.
+  Stride = SE.getMulExpr(SE.getNoopOrSignExtend(Coeff, WiderType),
----------------
Could you make it make more explicit that this is known-incorrect for some cases?
Suggestion:
```
// FIXME: This assumes that all values are signed integers which may be incorrect in unusual codes and incorrectly use sext instead of zext.
// for (uint32_t i = 256; i < 512; ++i) {
//   uint8_t trunc = i;
//   A[i] = 42;
// }
// This consecutively iterates twice over A. If `trunc` is sign-extended, we would conclude that this may iterate backwards over the array.
// However, LoopCacheAnalysis is heuristic anyway and transformations must not result in wrong optimizations if the heuristic was incorrect.
```


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

https://reviews.llvm.org/D128877