This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Rename getTied* methods to getMatching* in LinalgInterface.
ClosedPublic

Authored by olegshyshkov on Sep 23 2022, 7:30 AM.

Details

Summary

As mentioned in the comment to https://reviews.llvm.org/D134444, the term tied
is a misnomer in this context and matching sounds much better.

Diff Detail

Event Timeline

olegshyshkov created this revision.Sep 23 2022, 7:30 AM
Herald added a reviewer: aartbik. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
olegshyshkov requested review of this revision.Sep 23 2022, 7:30 AM

Generally LGTM, thanks for doing this @olegshyshkov !

Let's also see what @mravishankar thinks of this if you don't mind.

mlir/include/mlir/Dialect/Linalg/IR/LinalgInterfaces.td
422

getIndexingMapMatchingResult ?

Or just

getMatchingIndexingMap and rely on polymorphism to "make sense"?

873

This one is definitely "tied" not matching

Huh, I dont want to bikeshed on naming here. To me getTied* is more intuitive but it might also be familiarity bias. I dont have a strong enough opinion to suggest otherwise.

Resolved comments.

olegshyshkov marked an inline comment as done.Sep 27 2022, 5:46 AM
olegshyshkov added inline comments.
mlir/include/mlir/Dialect/Linalg/IR/LinalgInterfaces.td
422

Used getIndexingMapMatchingResult.

Compiler complained about duplicate definition of getMatchingIndexingMap, not sure why polymorphism didn't work.

olegshyshkov marked an inline comment as done.Sep 27 2022, 5:47 AM

clang-format

LG for sparse code, since this is just a mechanical name change, assuming consensus on the actual name is reached ;-)

nicolasvasilache accepted this revision.Sep 29 2022, 1:11 PM

Thanks Oleg!

This revision is now accepted and ready to land.Sep 29 2022, 1:11 PM

Sync and fix merge conflicts.

Thank you for reviewing!

This revision was landed with ongoing or failed builds.Sep 30 2022, 3:06 AM
This revision was automatically updated to reflect the committed changes.