[PATCH] D135808: [LoopInterchange] Correcting the profitability checking for vectorization

[Ramkrishnan Narayanan Komala via Phabricator via llvm-commits]

ram-NK added a comment.

In D135808#3865187 <https://reviews.llvm.org/D135808#3865187>, @bmahjour wrote:

> In D135808#3862874 <https://reviews.llvm.org/D135808#3862874>, @congzhe wrote:
>
>> In D135808#3861367 <https://reviews.llvm.org/D135808#3861367>, @ram-NK wrote:
>>
>>> If outer loop dependency direction  "=" and Inner loop dependency direction is "S" and "I" then, loop interchange is considered as profitable. Only two cases of dependency is profitable. But for vectorization, ">" and "<" dependency in outer loop is more profitable when interchanged. After patch [=,<] and [=,>] will be interchanged for vectorization.
>>
>> I thought what you meant is that after this patch, `[<, =]` and `[>, =]` (not [=,<] and [=,>]) will be interchanged? Because after interchange the dependency vector would become `[=, <]` and `[=, >]` respectively, which could improve potential parallelization and enable finer-grained parallelism, i.e., outer loop parallelism instead of inner loop parallelism. I think this is what `isProfitableForVectorization()` is supposed to be.
>>
>> I wonder if it makes sense to you @bmahjour ?
>
> I think profitability determination solely based on dependency matrix is fundamentally flawed. One obvious example is this
>
>   void f1() {
>     for (int i = 0; i < n; i++)
>       for (int j = 0; j < m; j++) {
>         B[j][i] = A[j][i];
>       }  
>   }
>
> Assuming A and B don't alias, there are no dependencies, nevertheless interchange is profitable.
>
> Another example is:
>
>   void f2() {
>     // > =
>     for (int i = 0; i < n; i++)
>       for (int j = 0; j < m; j++) {
>         ... = A[i][j];
>         A[i-1][j] = ...;
>       }
>   }
>
> The dependence is carried by the outer loop, yet it's not profitable to interchange (since it would make both locality and parallelism worse).
>
> The following example is profitable to interchange, but won't be recognized as profitable after this patch:
>
>   void f3() {
>     // = =
>     for (int i = 0; i < n; i++)
>       for (int j = 0; j < m; j++) {
>         ... = A[j][i];
>         A[j][i] = ...;
>       }
>   }

In loop interchange, profitablility is determined first by the cost model and then by `isProfitableForVectorization()`. Loops in f1() and f3() will be interchanged by `CacheCost` checking. In profitability checking of loop interchange, CacheCost analysis is done first. So this patch will not affect the f1() and f3() cases.

For f2() case, CacheCost and Legacy cost model will not show profitability. If loops in f2() is loop interchanged then, Outer loop parallelization is possible, which could improve parallelism.


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

https://reviews.llvm.org/D135808