[PATCH] D79850: [mlir] Extended BufferPlacement to support more sophisticated scenarios in which allocations cannot be moved freely and can remain in divergent control flow.

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: mlir/lib/Transforms/BufferPlacement.cpp:203
+      // (possibly existing) deallocation ops.
+      assert(allocateResultEffects.size() == 1 &&
+             "multiple allocations are not allowed");
----------------
dfki-mako wrote:
> herhut wrote:
> > This should not be an assert. Instead either ignore the `alloc` (which AFAIK was the mode before) or fail the transformation and report an error. 
> > 
> > Generally, the reason this was not supported before was that multiple results creates situations where the alloc cannot be moved. Can the new system not tolerate this with copies?
> > 
> > I am OK with this not being supported, as a multi-result `alloc` seems a very boutique use.
> In principal yes; however, we currently remove all `Dealloc` nodes in the beginning of the transformation. If we keep them for unsupported `Alloc` ops, this should not be an issue.
Can we be *safe* in this case instead of failing entirely?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79850