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

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added a comment.

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] = ...;
      }
  }




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

https://reviews.llvm.org/D135808