[PATCH] D28044: [LV/LoopAccess] Check statically if an unknown dependence distance can be proven larger than the loop-count

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

I did not understand why this works (I've added some questions inline).  Some "constructive" reasoning (not necessarily a proof) for why `isSafeDependenceDistance` is correct may help (i.e. we know X and therefore we know Y etc.).  If this is already documented / discussed elsewhere then a reference to that discussion / documentation will be helpful too.



================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1208
 
+/// Given a non-contant dependence-distance \p Dist between two accesses,
+/// that have the same stride \p Stride and the same type size \p TypeByteSize,
----------------
Nit: "non-constant"


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1225
+  // If we can prove that
+  //      (**) |Dist| > BackedgeTakenCount * Step
+  // then there is no dependence.
----------------
How does this work with a negative `BackedgeTakenCount * Step`?

For instance, in:

```
i = 0;
do {
  r0 = out[i + 1];
  out[i] = r1;
} while (--i != -2);
```

`D` is `1` (or `-1`), `BackedgeTakenCount` is `2` and `Step` is `-1`,
so `|Dist| > BackedgeTakenCount * Step` is true (I assume you intend
to use a signed `>` ?).

However, on the first iteration we write to `out[0]`, and we read from
it on the second iteration.

Or is this somehow not the kind of dependence you're looking for?


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1237
+  if (DistTypeSize > ProductTypeSize)
+    CastedProduct = SE.getZeroExtendExpr(Product, Dist.getType());
+  else
----------------
Can you add a comment for why you're zero extending `Product` but sign extending `Dist`?


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1241
+
+  // Is  Dist - (BackedgeTakenCount * Step) > 0 ?
+  // (If so, then we have proven (**) because |Dist| >= Dist)
----------------
How do you know that `Dist - (BackedgeTakenCount * Step)` won't
overflow?

I.e. why can't `Dist` be `i8 128` and `BackedgeTakenCount * Step` be
`i8 1`?

Skimming through LAA, it looks like it guards against overflow in the
addressing expressions themselves, but I think the situation above can
happen even if addressing expressions themselves do not overflow.

```
for (i8 i = 0; i < 2; i++) {
  r0 = out[i - 64];
  out[i + 64] = r1;
}
```



https://reviews.llvm.org/D28044