[PATCH] D77088: [mlir] [VectorOps] Implement a simple folder for identity vector.transpose operations.
Aart Bik via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 30 18:03:08 PDT 2020
aartbik accepted this revision.
aartbik added a comment.
Good to go, but some last comments to consider in the follow up.
================
Comment at: mlir/lib/Dialect/Vector/VectorOps.cpp:1536
+ // {0, 1, 2, ...}.
+ for (int64_t i = 0, e = transp.size(); i < e; i++) {
+ if (transp[i] != i)
----------------
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
================
Comment at: mlir/test/Dialect/Vector/canonicalize.mlir:155
+ %4 = vector.transpose %3, [0, 1, 2] : vector<2x3x4xf32> to vector<2x3x4xf32>
+ // CHECK-NEXT: return [[ADD]]
+ return %4 : vector<2x3x4xf32>
----------------
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)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77088/new/
https://reviews.llvm.org/D77088
More information about the llvm-commits
mailing list