This is an archive of the discontinued LLVM Phabricator instance.

[mlir] support memref of memref in standard-to-llvm conversion
ClosedPublic

Authored by ftynse on Jun 7 2021, 9:31 AM.

Details

Summary

Now that memref supports arbitrary element types, add support for memref of
memref and make sure it is properly converted to the LLVM dialect. The type
support itself avoids adding the interface to the memref type itself similarly
to other built-in types. This allows the shape, and therefore byte size, of the
memref descriptor to remain a lowering aspect that is easier to customize and
evolve as opposed to sanctifying it in the data layout specification for the
memref type itself.

Factor out the code previously in a testing pass to live in a dedicated data
layout analysis and use that analysis in the conversion to compute the
allocation size for memref of memref. Other conversions will be ported
separately.

Depends On D103827

Diff Detail

Event Timeline

ftynse created this revision.Jun 7 2021, 9:31 AM
ftynse requested review of this revision.Jun 7 2021, 9:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 7 2021, 9:31 AM
rriddle added inline comments.Jun 7 2021, 10:41 AM
mlir/include/mlir/Analysis/DataLayoutAnalysis.h
27–34

Do we need two methods here? Can we just have the getAtOrAbove(albeit with a different name) variant and require the user to do op->getParentOp()?

38

What is the size of DataLayout? Should we store in a unique_ptr instead?

mlir/lib/Analysis/DataLayoutAnalysis.cpp
27

Why not store the default layout as a separate field? (Removing the need to look it up).

mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
1917–1921

Can you add a comment on where the size is coming from here?

4185

Can you remove the mlir:: here?

ftynse updated this revision to Diff 350369.Jun 7 2021, 11:32 AM
ftynse marked 3 inline comments as done.

Address review.

ftynse marked an inline comment as done.Jun 7 2021, 11:33 AM
ftynse added inline comments.
mlir/include/mlir/Analysis/DataLayoutAnalysis.h
27–34

I expect getAbove to be used much more often than getAtOrAbove. The common request is "what is the data layout at the current operation, i.e. defined by its ancestors, not this op". Having to call getParentOp every time on it looks error-prone. getAtOrAbove is occasionally useful, and we can't assume that an op always has a child on which we could call getAbove.

38

Good idea!

mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
1917–1921

I can do better and put this next to the code that defines what a memref descriptor is, the size is coming from there.

rriddle accepted this revision.Jun 8 2021, 1:20 AM
This revision is now accepted and ready to land.Jun 8 2021, 1:20 AM
This revision was landed with ongoing or failed builds.Jun 8 2021, 2:11 AM
This revision was automatically updated to reflect the committed changes.