[PATCH] D28044: [LV/LoopAccess] Check statically if an unknown dependence distance can be proven larger than the loop-count
Dorit Nuzman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 31 02:30:49 PST 2017
dorit added a comment.
In https://reviews.llvm.org/D28044#661369, @sanjoy wrote:
> 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.
Thanks for looking closely into this.
I guess clarifying in the documentation that "Step" is the *absolute* stride would helpâ€¦ And I can also add the following:
"
We check here if the absolute distance (|Dist/Step|) is <= the loop iteration count.
This is equivalent to the Strong SIV Test (Practical Dependence Testing , Section 4.2.1);
Note, that for vectorization it is sufficient to prove that the dependence distance is >= VF; This is checked elsewhere.
But in some cases we can prune unknown dependence distances early, and even before selecting the VF, and without a runtime test, by comparing the distance against the loop iteration count. Since the vectorized code will be executed only if LoopCount >= VF, proving that the distance >= LoopCount also guarantees that distance >= VF."
================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1225
+ // If we can prove that
+ // (**) |Dist| > BackedgeTakenCount * Step
+ // then there is no dependence.
----------------
sanjoy wrote:
> 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?
Step cannot be negative. It is the absolute Stride in bytes. I will clarify that in the function description.
================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1237
+ if (DistTypeSize > ProductTypeSize)
+ CastedProduct = SE.getZeroExtendExpr(Product, Dist.getType());
+ else
----------------
sanjoy wrote:
> Can you add a comment for why you're zero extending `Product` but sign extending `Dist`?
Is this clear enough?:
"The dependence distance is signed (can be positive/negative), so we sign extend Dist; The (unsigned) product is the multiplication of the absolute stride in bytes, and the backdgeTakenCount, so we zero extend Product."
================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1241
+
+ // Is Dist - (BackedgeTakenCount * Step) > 0 ?
+ // (If so, then we have proven (**) because |Dist| >= Dist)
----------------
sanjoy wrote:
> 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;
> }
> ```
>
I'm not sure I understand the overflow concern... First of all, just to make sure it's clear: we reach this code only when the distance is unknown, so the specific example you gave with a known distance of 128 will not reach this code. So say the example was the following:
void foo (char *out, signed char Two, signed char SixtyFour) {
for (signed char i = 0; i < Two; i++) {
char r0 = out[i - SixtyFour];
out[i + SixtyFour] = r0 * 2;
}
}
The Dist that we will have is: (2 * (sext i8 %SixtyFour to i64))
The Minus expressions that we will compute are:
Minus = (0 + (2 * (sext i8 %SixtyFour to i64)) + (-1 * (zext i8 %Two to i64))<nsw>)
Minus = (0 + (-2 * (sext i8 %SixtyFour to i64)) + (-1 * (zext i8 %Two to i64))<nsw>)
(And of course in this case we will not be able to prove statically that this is positive, so we will return that the distance is not safe).
https://reviews.llvm.org/D28044
More information about the llvm-commits
mailing list