[PATCH] D108371: [LAA] Add Memory dependence and unknown bounds remarks.

[Malhar via Phabricator via llvm-commits]

malharJ added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LoopAccessAnalysis.h:261
 
+  const SmallVector<Dependence, 2> &getUnsafeDependences() const {
+    return UnsafeDependences;
----------------
fhahn wrote:
> There's no need to return a SmallVector with the length encoded, the callers should not care about that. You can use ArrayRef if the callers do not add to the vector or SmalLVectorImpl if they need to add elements.
Currently the code uses "auto" when accessing the. value returned by this getter
so there isn't a need for the user to know the size used in the template ...

regardless, I've changed it to use SmallVector<T> instead if that's ok.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:590
+  //
+  // Used when emitting unknown_array_bounds remark.
+  SmallPtrSet<Value *, 4> UncomputablePtrs;
----------------
fhahn wrote:
> Use `///` for all comments. Also, does this need to be public after the recent changes?
It was being accessed from outside the class on line 2045.

I've moved it to private now and added a public getter instead.



================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1721
 
+            // Runtime checks are only feasible, if all unsafe dependencies are
+            // unknown. For other unsafe deps, we already know they will fail
----------------
fhahn wrote:
> Why call it `UnsafeDependences` if it is supposed to only contain unknown dependences?
I  think UnsafeDependences contains both unknown and known unsafe dependences.
I think the comment is unclear so I'm removing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108371