[PATCH] D72168: [mlir][Linalg] Add a linalg.reshape op

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache marked 5 inline comments as done.
nicolasvasilache added inline comments.


================
Comment at: mlir/include/mlir/IR/StandardTypes.h:12
 
+#include "mlir/IR/AffineExpr.h"
 #include "mlir/IR/Types.h"
----------------
rriddle wrote:
> This should be avoided as much as possible. We should avoid adding more includes to headers, especially common ones.
ugh .. of course, thanks!


================
Comment at: mlir/lib/IR/StandardTypes.cpp:715
+    auto maps = llvm::to_vector<4>(affineMaps);
+    for (auto &m : maps)
+      m = simplifyAffineMap(m);
----------------
rriddle wrote:
> Why do these have to be re-simplified? Seems like this could just be done on construction, what is the benefit of doing this here?
I can avoid this and just return the original type for now.
I do not remember the rationale for why `simplifyAffineMap` cannot be run by default but I remember opposition 1+ year ago.
It may be related to roundtripping and old ways of writing tests. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72168