[mlir] [VectorOps] Implement a simple folder for identity vector.transpose operations.

[grosul1]

Summary

[VectorOps] Implement a simple folder for identity vector.transpose operations.

[nicolasvasilache]

Is there a followup revision planned in which you would canonicalize transpose(transpose()) and have it compose with these newly added folders?

[rriddle]

mlir/lib/Dialect/Vector/VectorOps.cpp

1540

Please run clang-format.

[aartbik]

please label title with mlir as well

[mlir] [VectorOps]

mlir/include/mlir/Dialect/Vector/VectorOps.td

1318

note that the attribute is really called "transp" (perhaps not such a good name looking back), so getTransp or getTranspArray would be slightly more in style; but I am okay with this too and perhaps rename the array later

mlir/lib/Dialect/Vector/VectorOps.cpp

1535

wouldn't it read a little easier as

for (int64_t i = 0, n = permutation.size; i < n; i++)

if (permutation[i] != i) 
...

i.e. less casting?

mlir/test/Dialect/Vector/canonicalize.mlir

132

maybe for completeness of what you have right now also a combination of transpose-ID and transpose-non-ID to make sure only the latter remain?

[grosul1]

In D77088#1951080, @nicolasvasilache wrote:

Is there a followup revision planned in which you would canonicalize transpose(transpose()) and have it compose with these newly added folders?

Thanks, Nicolas, yes, I expect to make an update to eliminate combinations of transposes.

[grosul1]

removeme

[grosul1]

mlir/include/mlir/Dialect/Vector/VectorOps.td

1318

Done! Renamed everything to transp/Transp, no discrepancy for now. Everything can be renamed at once if/when someone gets to do it.

mlir/lib/Dialect/Vector/VectorOps.cpp

1535

Used "e = " as it seems to be a convention; in this file, at least.

[aartbik]

Good to go, but some last comments to consider in the follow up.

mlir/lib/Dialect/Vector/VectorOps.cpp

1536

Given the llvm style of leaving the {} out of trivial for/ifs (something I am not a big fan of), I am wondering if that actually also applies to this case, trivial for loop with trivial if, so no {} on for either; perhaps others will comment, otherwise just leave it

mlir/test/Dialect/Vector/canonicalize.mlir

155

this of course captures that the intermediate tranpose is gone, but to be completely consistent with your other tests, you would expect a CHECK-NOT here (to make sure it is also removed as dead code)

[grosul1]

Update the test

[grosul1]

mlir/lib/Dialect/Vector/VectorOps.cpp

1536

I can turn this loop (back) into a lambda.
Not quite the same case, but similar: https://llvm.org/docs/CodingStandards.html#turn-predicate-loops-into-predicate-functions.

[grosul1]

mlir/lib/Dialect/Vector/VectorOps.cpp

1536

Submitting as is.

[grosul1]

Refresh