[PATCH] D78484: Providing buffer assignment for MLIR

[Marcel Koester via Phabricator via llvm-commits]

dfki-mako added inline comments.


================
Comment at: mlir/lib/Transforms/BufferAssignment.cpp:368
+      }
+    });
+  };
----------------
mehdi_amini wrote:
> dfki-mako wrote:
> > mehdi_amini wrote:
> > > Isn't this all assuming a particular model with a single allocation / deallocation per buffer?
> > > What about a pattern like the following with two allocs feeding into one dealloc? (we can imagine more complex cases)
> > > 
> > > ```
> > >     cond_br %cond, ^bb1, ^bb2
> > >   ^bb1:
> > >     %buffer1 = alloc ...
> > >     br ^exit(%buffer1 : ....)
> > >   ^bb2:
> > >     %buffer2 = alloc ...
> > >     br ^exit(%buffer2 : ...)
> > >   ^exit(%buffer: ...):
> > >     dealloc %buffer
> > > ```
> > This pass currently assumes a single-allocation/deallocation model that usually appears during straight-forward legalization (lowering) of operations. We wanted to keep the first version simple and would like to significantly extend the functionality in one of the follow-up CLs.
> Sure, but please document this restriction in the TableGen pass description and add a TODO in the code so the reader is aware that this is a known limitation 
> (in general it helps me reviewing code when the known limitations are spelled out and there is explicit acknowledgement of what will be addressed in followup revisions)
> 
> Even in single-allocation/deallocation, would the pass pessimizes case of conditional allocation? For example in the following (pseudo) code I suspect you'd increase the dynamic memory consumption by always allocating both buffers:
> 
> ```
>   if (cond)
>    %buffer1 = alloc ...
>   else
>    %buffer2 = alloc ...
>   ...
>   if (cond) {
>    consume(buffer1)
>    dealloc(buffer1)
>   } else {
>    consume(buffer2)
>    dealloc(buffer2)
>   }
> ```
> 
> 
In this case, the current allocation policies will cause the dynamic memory allocation to increase, as you outlined above. However, we are going to extend the documentation to describe the current limitations and assumptions. In one of the future CLs, the buffer placement transformation will include several optimization passes to optimize the overall memory consumption.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78484