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

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache accepted this revision.
nicolasvasilache added inline comments.
This revision is now accepted and ready to land.


================
Comment at: mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp:15
 #include "mlir/Dialect/PDL/IR/PDL.h"
+#include "mlir/Dialect/SCF/IR/SCF.h"
 #include "mlir/Dialect/Transform/IR/TransformDialect.h"
----------------
qcolombet wrote:
> I'm guessing I need to modify some bazel files because of this new dependency, right?
> 
> (And looks like I dropped the CMakeFiles.txt changes as well and nothing broke x))
It is likely.

I use this to run locally for bazel : https://github.com/nicolasvasilache/nicolas.vasilache.github.io/blob/master/.venv/mlirdev/bin/activate#L64

That catch 99% of what I usually need.




================
Comment at: mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp:35
+    for (Operation *user : target->getUsers()) {
+      auto loop = user->getParentOfType<scf::ForOp>();
+      if (!loop) {
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143747