[PATCH] D75920: [LoopCacheAnalysis] Improve cost heuristic.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: llvm/lib/Analysis/LoopCacheAnalysis.cpp:106-119
+  const SCEV *TripCount =
+      (!isa<SCEVCouldNotCompute>(BackedgeTakenCount) &&
+       isa<SCEVConstant>(BackedgeTakenCount))
+          ? SE.getAddExpr(BackedgeTakenCount,
+                          SE.getOne(BackedgeTakenCount->getType()))
+          : nullptr;
+
----------------
[sugggestion]
```
if (auto *TripCount = dyn_cast<SCEVConstant>(BackedgeTakenCount)) 
  return SE.getAddExpr(BackedgeTakenCount, SE.getOne(BackedgeTakenCount->getType())

LLVM_DEBUG(dbgs() << "Trip count of loop " << L.getName() << " could not be computed, using DefaultTripCount\n");
return SE.getConstant(ElemSize.getType(), DefaultTripCount);
```



================
Comment at: llvm/lib/Analysis/LoopCacheAnalysis.cpp:107
+  const SCEV *TripCount =
+      (!isa<SCEVCouldNotCompute>(BackedgeTakenCount) &&
+       isa<SCEVConstant>(BackedgeTakenCount))
----------------
`isa<SCEVCouldNotCompute>` is redundant: the object cannot be `SCEVCouldNotCompute` and `SCEVConstant` at the same time


================
Comment at: llvm/lib/Analysis/LoopCacheAnalysis.cpp:452
 
+bool IndexedReference::getSubscriptIndex(const Loop &L, unsigned &Index) const {
+  for (auto Idx : seq<unsigned>(0, getNumSubscripts())) {
----------------
What if:
 * Multiple subscripts are using the loop variables?
 * A subscript uses multiple loop variables (eg. `i+j`)?
 * `SCEVAddRecExpr` is processed further, e.g. negated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75920