This is an archive of the discontinued LLVM Phabricator instance.

[mlir][linalg] move isElementwise() to Linalg/Utils (NFC)
ClosedPublic

Authored by okkwon on Jun 22 2022, 4:13 PM.

Diff Detail

Event Timeline

okkwon created this revision.Jun 22 2022, 4:13 PM
Herald added a project: Restricted Project. · View Herald Transcript
okkwon requested review of this revision.Jun 22 2022, 4:13 PM
aartbik accepted this revision.Jun 22 2022, 4:21 PM
aartbik added inline comments.
mlir/lib/Dialect/Linalg/Utils/Utils.cpp
152

just looking at the style in this file, it seems to make sense to move this static method up, and keep this region for just the public API, but the end result is the same, so up to you....

This revision is now accepted and ready to land.Jun 22 2022, 4:21 PM
aartbik requested changes to this revision.Jun 22 2022, 4:25 PM
aartbik added inline comments.
mlir/lib/Dialect/Linalg/Utils/Utils.cpp
22

This change pulls in a dependence for Linalg.

Don't you need a CMake and Bazel file change for this?

This revision now requires changes to proceed.Jun 22 2022, 4:25 PM
okkwon updated this revision to Diff 439209.Jun 22 2022, 4:37 PM
  • Expose hasOnlyScalarElementwiseOp as a global function.
  • Specify a dependency to FuncDialect in the CMake and Bazel files.

Thanks for the review. BTW, how can I test the changes on the cmake and bazel files? I usually run "ninja check-mlir" for testing.

okkwon marked 2 inline comments as done.Jun 22 2022, 4:39 PM

Thanks for the review. BTW, how can I test the changes on the cmake and bazel files? I usually run "ninja check-mlir" for testing.

I always find building with -DBUILD_SHARED_LIBS=ON the config a very good way of making sure no dependencies are missed.

aartbik accepted this revision.Jun 22 2022, 4:42 PM
aartbik added inline comments.
mlir/include/mlir/Dialect/Linalg/Utils/Utils.h
38

It is a bit strange to refer to r now that you removed the parameter name from the prototype

mlir/lib/Dialect/Linalg/Utils/Utils.cpp
152

Ah, making this part of the public API is another way to "fix" this :-)

This revision is now accepted and ready to land.Jun 22 2022, 4:42 PM
okkwon updated this revision to Diff 439214.Jun 22 2022, 4:47 PM

Recover the missing r.

okkwon marked 2 inline comments as done.Jun 22 2022, 4:52 PM
okkwon added inline comments.
mlir/include/mlir/Dialect/Linalg/Utils/Utils.h
38

It is not intended. Recovered it. Thanks for catching it!