Page MenuHomePhabricator

[mlir][Linalg] Add support for scf::ForOp in comprehensive bufferization (7/n)
ClosedPublic

Authored by nicolasvasilache on Jun 17 2021, 2:46 PM.

Details

Summary

scf::ForOp bufferization analysis proceeds just like for any other op (including FuncOp) at its boundaries; i.e. if:

  1. The tensor operand is inplaceable.
  2. The matching result has no subsequent read (i.e. all reads dominate the scf::ForOp).
  3. In and does not create a RAW interference.

then it can bufferize inplace.

Still there are a few differences:

  1. bbArgs for an scf::ForOp are always considered inplaceable when seen from ops inside the body. This is because a) either the matching tensor operand is not inplaceable and an alloc will be inserted (which makes bbArg itself inplaceable); or b) the tensor operand and bbArg are both already inplaceable.
  2. Bufferization within the scf::ForOp body has implications to the outside world : the scf.yield terminator may well ping-pong values of the same type. This muddies the water for alias analysis and is not supported atm. Such cases result in a pass failure.

Diff Detail

Event Timeline

nicolasvasilache requested review of this revision.Jun 17 2021, 2:46 PM
ftynse accepted this revision.Jun 21 2021, 3:49 AM
ftynse added inline comments.
mlir/include/mlir/Dialect/SCF/SCFOps.td
298

Copy-pasta from above, There's no such assertion here.

mlir/lib/Dialect/Linalg/Transforms/ComprehensiveBufferize.cpp
210

Why drop the extra doc?

269

typo: "optiona"

1285–1286

If we bail, should this return failure() rather than continue?

1741–1743

Nit: LogicalResult implicitly converts to WalkResult so return bufferizationSanityCheck(yieldOp, aliasInfo) should work here.

This revision is now accepted and ready to land.Jun 21 2021, 3:49 AM
nicolasvasilache marked 5 inline comments as done.

Address comments.

Rebase and fix build.

This revision was landed with ongoing or failed builds.Jun 24 2021, 8:04 AM
This revision was automatically updated to reflect the committed changes.