[VectorOps] Implement a simple folder for identity vector.transpose operations.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Is there a followup revision planned in which you would canonicalize transpose(transpose()) and have it compose with these newly added folders?
| mlir/lib/Dialect/Vector/VectorOps.cpp | ||
|---|---|---|
| 1540 | Please run clang-format. | |
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? | |
Thanks, Nicolas, yes, I expect to make an update to eliminate combinations of transposes.
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) | |
| mlir/lib/Dialect/Vector/VectorOps.cpp | ||
|---|---|---|
| 1536 | I can turn this loop (back) into a lambda. | |
| mlir/lib/Dialect/Vector/VectorOps.cpp | ||
|---|---|---|
| 1536 | Submitting as is. | |
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