Page MenuHomePhabricator

[mlir] Add func-bufferize pass.
ClosedPublic

Authored by silvas on Oct 26 2020, 6:00 PM.

Details

Summary

This is the most basic possible finalizing bufferization pass, which I
also think is sufficient for most new use cases. The more concentrated
nature of this pass also greatly clarifies the invariants that it
requires on its input to safely transform the program (see the
pass description in Passes.td).

With this pass, I have now upstreamed practically all of the
bufferizations from npcomp (the exception being std.constant, which can
be upstreamed when std.global_memref lands:
https://llvm.discourse.group/t/rfc-global-variables-in-mlir/2076/16 )

Diff Detail

Event Timeline

silvas created this revision.Oct 26 2020, 6:00 PM
silvas requested review of this revision.Oct 26 2020, 6:00 PM

Loos good, thanks @silvas !

This revision is now accepted and ready to land.Oct 30 2020, 12:23 PM

@dfki-mako , @herhut , I'd like for one of you to approve this one.

The overall picture is the users will be able to run pipelines like -scf-bufferize -std-bufferize -linalg-bufferize -func-bufferize. The func-bufferize will terminate the pass pipeline and "finalize" the conversion. That will then complete the picture for composable bufferizations.

With the changes discussed in https://llvm.discourse.group/t/rfc-simplifying-bufferizetypeconverter/2091/2 we can then really trim down the current TestBufferPlacementPreparation pass (possibly deleting it depending on how much we can simplify it).

dfki-mako added inline comments.Oct 30 2020, 4:07 PM
mlir/include/mlir/Dialect/StandardOps/Transforms/Passes.td
28

Question: std.func, std.call and std.return ops? The actual code inside the pass doesn't adapt any return ops, does it?

38

missing to -> to be converted?

42

Suggestion: bufferize -> bufferize step?

mlir/lib/Dialect/StandardOps/Transforms/FuncBufferize.cpp
2

Bufferize.cpp -> FuncBufferize.cpp

silvas marked 3 inline comments as done.Oct 30 2020, 6:02 PM
silvas added inline comments.
mlir/include/mlir/Dialect/StandardOps/Transforms/Passes.td
28

Dialect conversion patterns are mostly about replacing results. Operand types change naturally as their definitions change type. So std.return is not explicitly handled or mentioned here, because it doesn't have results.

In the case of "non-finalizing" bufferize conversions, terminators like scf.yield need patterns so that they create materializations to the target type. But that is not here.

We could write this pass as a "non-finalizing" (also known as "composable") bufferize pass, in which case we would need it. However, as I describe below, because block arguments are rewritten, it is natural for this pass to be finalizing so that successor operands of terminators are all handled naturally. For example, we don't need patterns for std.br, std.cond_br or any other terminator that passes values to successors. If this was a "composable" bufferize pass, then we would need patterns for *every* possible terminator (and not all terminators implement BranchOpInterface for us to guarantee we can rewrite them generically).

38

I think it is right as-is. "requires that X be converted", for X = "all successor arguments of predecessors". (I think you would be correct if the "that" was removed).

silvas updated this revision to Diff 302053.Oct 30 2020, 6:03 PM
silvas marked 2 inline comments as done.

Address comments

pifon2a accepted this revision.Nov 2 2020, 1:39 AM
pifon2a added a subscriber: pifon2a.
pifon2a added inline comments.
mlir/test/Dialect/Standard/func-bufferize.mlir
7

nit: remove CHECK for '}'. Here and below.

25

nit: reduce indentation to fit 80 chars? here and below

This revision was automatically updated to reflect the committed changes.
silvas marked 2 inline comments as done.