This is an archive of the discontinued LLVM Phabricator instance.

[mlir][bufferize] Rename BufferAssignment* to Bufferize*
ClosedPublic

Authored by silvas on Oct 12 2020, 2:37 PM.

Diff Detail

Event Timeline

silvas created this revision.Oct 12 2020, 2:37 PM
silvas requested review of this revision.Oct 12 2020, 2:37 PM
herhut accepted this revision.Oct 13 2020, 12:57 AM

It would be great to have more consistency wrt. naming of patterns and passes but this is a step in the right direction.

Left comments for further improvements but those need not be in this change.

mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h
801

Why are these in a header file?

807

Should we aim for a somewhat standardized name for the patterns themselves? Like BufferizeLinalgOpConverter or LinalgOpBufferizer?

mlir/include/mlir/Dialect/Shape/Transforms/Passes.h
43

Do we keep the name TypeConversionPatterns? Or should this also become BufferizeOpConversionPatterns? I think the latter is also already in use.

mlir/lib/Dialect/Linalg/Transforms/Bufferize.cpp
125

This comment is off.

mlir/lib/Dialect/Shape/Transforms/Bufferize.cpp
23

Could this also be renamed?

77

Here, too. Also scrap the TODO?

The BufferizeTypeConverter allows to configure how types should be rewritten.

dfki-mako accepted this revision.Oct 13 2020, 4:33 AM
dfki-mako added inline comments.
mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h
807

@herhut sounds good to me. Maybe we should aim for LinalgOpBufferizer.

mlir/include/mlir/Dialect/Shape/Transforms/Passes.h
43

I guess it makes sense to unify these names in a way that they express their association to the internal Bufferize functionality.

mlir/lib/Dialect/Linalg/Transforms/Bufferize.cpp
125

This comment is out-of-date and should be adapted.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 14 2020, 12:41 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.