[PATCH] D79829: [mlir][Affine][NFCI]: Introduce affine memory interfaces

[Alex Zinenko via Phabricator via llvm-commits]

ftynse added a comment.

LGTM in general.

I am wondering whether `AffineMemoryOpInterfaces` belongs in `lib/Interfaces` since it is specific to the Affine dialect. I'd rather put it together with the dialect, even though the transformations live in `lib/Transforms` .



================
Comment at: mlir/include/mlir/Dialect/Affine/IR/CMakeLists.txt:2
 add_mlir_dialect(AffineOps affine)
+#TODO: Add MLIRAffineMemoryOpInterfacesIncGen dependency
 add_mlir_doc(AffineOps -gen-op-doc AffineOps Dialects/)
----------------
Please do :)


================
Comment at: mlir/lib/Analysis/AffineAnalysis.cpp:665
     map = loadOp.getAffineMap();
-  else if (auto storeOp = dyn_cast<AffineStoreOp>(opInst))
+  else {
+    auto storeOp = cast<AffineWriteLikeOpInterface>(opInst);
----------------
Nit: please make braces symmetric


================
Comment at: mlir/lib/Transforms/LoopFusion.cpp:329
+      if (getOutEdgeCount(
+              id, cast<AffineWriteLikeOpInterface>(storeOpInst).getMemRef()) >
+          0)
----------------
Nit: extract `cast<AffineWriteLikeOpInterface>(storeOpInst).getMemRef()` into a named variable for better formatting here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79829