This is an archive of the discontinued LLVM Phabricator instance.

[mlir][tensor/memref] Replace various redundant helpers with `reifyShapeDims`
AcceptedPublic

Authored by springerm on Mar 14 2023, 8:10 AM.

Diff Detail

Event Timeline

springerm created this revision.Mar 14 2023, 8:10 AM
Herald added a reviewer: aartbik. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
springerm requested review of this revision.Mar 14 2023, 8:10 AM
springerm added a comment.EditedMar 14 2023, 8:13 AM

Note: This revision is separate from D146052 because it contains mostly cases where the exact type of the shaped value is known. I.e., in most cases we know whether a tensor.dim or a memref.dim should be generated. Going through reifyShapeDims saves a few lines of code each time, but it also goes through the interface method each time (which has some overhead). It's a tradeoff. Therefore, putting these change separately.

(We could still de-duplicate some parts of the code base a bit, even if we don't go the interface.)

LG for sparse with one nit

mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp
121

please use

for (int64_t dim = 0, rank = getRank(); dim < rank; ++dim)

idiom we always use in sparse compiler module

mehdi_amini added inline comments.Mar 14 2023, 10:54 AM
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp
121

If you like functional style, you also have: for (int64_t dim : llvm::seq(0, getRank())) :)

mravishankar accepted this revision.Apr 19 2023, 9:34 AM

This looks good to me. I am approving this change. I dont have comments on the depedent patch though. If that is good, this is good for me.

This revision is now accepted and ready to land.Apr 19 2023, 9:34 AM
dcaballe accepted this revision.Apr 19 2023, 11:11 AM