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

[Aart Bik via Phabricator via llvm-commits]

aartbik added a comment.

please label title with mlir as well

[mlir] [VectorOps]



================
Comment at: mlir/include/mlir/Dialect/Vector/VectorOps.td:1318
     }
+    void getPermutation(SmallVectorImpl<int64_t> &results);
   }];
----------------
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


================
Comment at: mlir/lib/Dialect/Vector/VectorOps.cpp:1535
+  // Check if the permutation contains sequential values: {0, 1, 2, ...}.
+  for (size_t i = 0; i < permutation.size(); i++) {
+    if (permutation[i] != static_cast<int64_t>(i))
----------------
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?


================
Comment at: mlir/test/Dialect/Vector/canonicalize.mlir:132
+  %1 = vector.transpose %0, [0, 1, 2] : vector<4x3x2xf32> to vector<4x3x2xf32>
+  // CHECK-NEXT: return [[ARG]]
+  return %1 : vector<4x3x2xf32>
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77088