[PATCH] D132703: [LAA] Use BTC to rule out dependences if one ptr is loop invariant.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1873
+  uint64_t Stride = -1;
+  if (StrideAPtr && StrideBPtr && StrideAPtr == StrideBPtr) {
+    Stride = std::abs(StrideAPtr);
----------------
reames wrote:
> Not sure if this is a generalization, or a possible bug, but the fact you're dependent on the loop invariant bit here looks off.
> 
> My reasoning is as follows, if we know the stride of either access and the trip count, we can describe the region which that pointer can access.  It shouldn't matter whether the other region is fixed size (i.e. pointer is loop invariant), or variably sized if we can prove that one region is before or after another.
> 
> Unless maybe we're trying to reason about the case where we don't know the sign of the distance between the start of the two regions?  
> 
> Knowing that either operand is loop invariant does tell you that region is small.  I could see how that might be useful when we can prove distance is far from zero (but potentially negative.)
> 
> However, in that case, I think there's a missing check to prove that the invariant region is smaller than abs(distance).  That is, that the access type is smaller than distance.  If it's not, we could have one varying region which starts with one byte offset into the loop invariant region.  
> 
> Honestly, I think this code is made far more confusing by the fact that 0 is used an error value for stride.  0 is a valid stride; we really should be using Optional here instead.  
> 
> 
Thanks, I don't think loop invariance is strictly necessary for correctness, just to limit the initial scope of the implementation. Let me look into writing up some test cases and see if it can be easily extended in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132703