This is an archive of the discontinued LLVM Phabricator instance.

[mlir][NFC] Cleanup: Move helper functions to StaticValueUtils
ClosedPublic

Authored by springerm on Jun 21 2021, 10:21 PM.

Details

Summary

Reduce code duplication: Move various helper functions, that are duplicated in TensorDialect, MemRefDialect, LinalgDialect, StandardDialect, into a new StaticValueUtils.cpp.

Depends On D104676

Diff Detail

Event Timeline

springerm created this revision.Jun 21 2021, 10:21 PM
springerm requested review of this revision.Jun 21 2021, 10:21 PM
rriddle requested changes to this revision.Jun 21 2021, 10:33 PM
rriddle added inline comments.
mlir/lib/Dialect/Utils/StaticValueUtils.cpp
11

Why does this depend on Standard ops?

40

nit: Drop the llvm:: here.

49

Drop the llvm:: here and several other places.

52–53

Use m_Constant here, and then you can drop the dependency on StandardOps.

59–62

Is this method useful? It can be represented with just ==.

63–64
76
This revision now requires changes to proceed.Jun 21 2021, 10:33 PM

update CMakeFiles

springerm updated this revision to Diff 353827.Jun 22 2021, 5:38 PM
springerm marked 7 inline comments as done.

address comments

springerm added inline comments.Jun 22 2021, 5:39 PM
mlir/lib/Dialect/Utils/StaticValueUtils.cpp
59–62

Nice, I didn't think of that.

rriddle accepted this revision.Jun 24 2021, 11:21 AM

LGTM, thanks!

mlir/include/mlir/Dialect/Linalg/IR/LinalgOps.td
272 ↗(On Diff #353827)

Is the mlir:: necessary on these?

mlir/include/mlir/Dialect/Utils/StaticValueUtils.h
20

nit: Remove the newline here.

mlir/lib/Dialect/Utils/StaticValueUtils.cpp
12

nit: Drop the newline here.

15

nit: We traditionally keep llvm::None spelled out. Optional and SmallVector should already be exported in the mlir namespace (in mlir/Support/LLVM.h)

41

nit: Spell out auto here.

This revision is now accepted and ready to land.Jun 24 2021, 11:21 AM
This revision was landed with ongoing or failed builds.Jun 26 2021, 11:57 PM
This revision was automatically updated to reflect the committed changes.
springerm marked 5 inline comments as done.