[PATCH] D77320: [MLIR] fix/update affine data copy utility for max/min bounds

[Diego Caballero via Phabricator via llvm-commits]

dcaballe added inline comments.


================
Comment at: mlir/lib/Dialect/Affine/Transforms/AffineDataCopyGeneration.cpp:276
+  AffineStoreOp::getCanonicalizationPatterns(patterns, &getContext());
+  applyPatternsGreedily(f, std::move(patterns));
 }
----------------
I'm not asking for any changes now but just wondering if it would make sense in the future to do all of these "clean-up" optimizations in a separate pass(es) that we can invoke as needed, maybe after running a bunch of optimizations instead of trying to optimize right after each one if it's not absolutely necessary. I guess that could reduce compile time and avoid duplicating this clean-up per pass. IIRC, loop fusion performed also some optimization around temporary tensors after fusion. Not sure if that optimizations would also fit into this category.  


================
Comment at: mlir/lib/Transforms/Utils/LoopUtils.cpp:1488
+  unsigned rank = memref.getType().cast<MemRefType>().getRank();
+  (void)rank; // unused in opt mode
+  assert(lbMaps.size() == rank && "wrong number of lb maps");
----------------
used in line 1496


================
Comment at: mlir/test/Dialect/Affine/affine-data-copy.mlir:114
 //  FILTER-NOT:   alloc()
-//      FILTER:   affine.for %{{.*}} = 0 to 128 {
+//      FILTER:   affine.for %{{.*}}
 //      FILTER:     affine.for %{{.*}} = 0 to 4096 {
----------------
nit: you can remove the `%{{.*}}`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77320