[PATCH] D63459: Loop Cache Analysis

Michael Kruse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 17 13:22:07 PDT 2019


Meinersbur accepted this revision.
Meinersbur added a comment.
This revision is now accepted and ready to land.

LGTM, some nitpicks left.



================
Comment at: llvm/include/llvm/Analysis/LoopCacheAnalysis.h:181
+public:
+  static CacheCostTy constexpr InvalidCost = ULLONG_MAX;
+
----------------
`CacheCostTy` is `int64_t`; did you mean to assign `-1`?


================
Comment at: llvm/include/llvm/Analysis/LoopCacheAnalysis.h:200
+
+  const SmallVector<LoopCacheCostTy, 3> &getLoopCosts() const {
+    return LoopCosts;
----------------
To not leak the internal list implementation, return `ArrayRef<LoopCacheCostTy>`.


================
Comment at: llvm/lib/Analysis/LoopCacheAnalysis.cpp:167
+
+  auto D = DI.depends(&StoreOrLoadInst, &Other.StoreOrLoadInst, true);
+  if (D == nullptr) {
----------------
[[ https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable | Don’t “almost always” use auto ]]


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63459





More information about the llvm-commits mailing list