[PATCH] D143747: [mlir][MemRef][Transform] Don't apply multibuffer on "useless" allocs

Mon Feb 13 00:08:34 PST 2023

qcolombet added inline comments.

================
Comment at: mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp:35
+    for (Operation *user : target->getUsers()) {
+      auto loop = user->getParentOfType<scf::ForOp>();
+      if (!loop) {
----------------
nicolasvasilache wrote:
> I haven't looked at the impl of multi-buffering but I doubt it is general enough to support arbitrary containing ops on the way to the first scf::ForOp ?
> 
> Maybe use:
> ```
>  auto loop = dyn_cast_or_null<scf::ForOp>(user->getParentOp());
> ```
> ?
> 
> This seems in line with your tests in which the scf.for is always the immediate parent.
Good catch, I went with the IREE filtering but the multi-buffering implementation is actually more general.
https://github.com/iree-org/iree/blob/a1c4b9313c561c9f80d6e15ef2f01742b5bddca6/compiler/src/iree/compiler/Codegen/Common/GPUMultiBuffering.cpp#L33

I'm going to match what the multi-buffering implementation does.
Longer term we should refactor the implementation of multi-buffering to not duplicate the implementation.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143747