[PATCH] D79658: [mlir][Affine] Introduce affine.vload and affine.vstore

[Diego Caballero via Phabricator via llvm-commits]

dcaballe marked 3 inline comments as done.
dcaballe added a comment.

A few things I found along the road...



================
Comment at: mlir/include/mlir/Conversion/Passes.td:18
 
 def ConvertAffineToStandard : FunctionPass<"lower-affine"> {
   let summary = "Lower Affine operations to a combination of Standard and Loop "
----------------
Should we rename this to `convert-affine-to-loops`?


================
Comment at: mlir/include/mlir/Dialect/Affine/IR/AffineOps.td:817
+  // extraClassDeclaration. Is there a way to add more declarations without
+  // overriding AffineLoadOpBase's.
+  // let extraClassDeclaration = [{
----------------
Not sure if it's feasible at all in tablegen but it would be great if we could make "code" variables (`extraClassDeclaration`, `builders`, etc.) "additive" in a hierarchy instead of overriding the variable with the value from the subclass.


================
Comment at: mlir/lib/Conversion/AffineToVector/AffineToVector.cpp:55
+        op, op.getVectorType(), op.getMemRef(), *resultOperands,
+        AffineMapAttr::get(permMap), padding);
+    return success();
----------------
I would be great if we could have a builder that sets `getMinorIdentityMap` as the default value of the permutation map for those cases where no permutation is needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79658