[PATCH] D108763: Use type sizes when determining dependence

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

Hi @jolanta.jensen thanks for this change!



================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1526
   uint64_t TypeByteSize = DL.getTypeAllocSize(ATy);
+  bool SizesAreSame = TypeByteSize == DL.getTypeAllocSize(BTy);
   uint64_t Stride = std::abs(StrideAPtr);
----------------
nit: s/SizesAreSame/HasSameSize/
(this may be personal preference, so feel free to ignore).


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1530
   if (!C) {
-    if (!isa<SCEVCouldNotCompute>(Dist) &&
-        TypeByteSize == DL.getTypeAllocSize(BTy) &&
+    if (!isa<SCEVCouldNotCompute>(Dist) && SizesAreSame &&
         isSafeDependenceDistance(DL, *(PSE.getSE()),
----------------
Is it worth also having a negative and positive test for this case, one where the sizes are not the same (guaranteed dependence) and one where the sizes are the same (no dependence). I think we at least need a positive test (i.e. proving no dependence when the types are different, but the same size, because I don't think your current tests covers the case of an unknown dependence distance).


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1545
   // Attempt to prove strided accesses independent.
-  if (std::abs(Distance) > 0 && Stride > 1 && ATy == BTy &&
+  if (std::abs(Distance) > 0 && Stride > 1 && SizesAreSame &&
       areStridedAccessesIndependent(std::abs(Distance), Stride, TypeByteSize)) {
----------------
same question for this case.


================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/depend_diff_types.ll:43
+
+    ;; Store to vec[indvars.iv] as i64, creating a backwards dependency between
+    ;; the two stores with different element types but the same element size.
----------------
nit: s/backwards/backward


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108763